Skip to content

chore(): add updated snapshots #27561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jul 7, 2023

Conversation

brandyscarney
Copy link
Member

@brandyscarney brandyscarney commented May 26, 2023

This PR contains the screenshots for #27547

See this comment thread here for why there are so many diffs with this change.

@bolt-new-by-stackblitz
Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@github-actions github-actions bot added the package: core @ionic/core package label May 26, 2023
@brandyscarney brandyscarney changed the base branch from FW-1808-wrap to main June 14, 2023 21:23
@brandyscarney brandyscarney force-pushed the FW-1808-wrap-update-screenshots branch from e766af3 to 753c24c Compare June 14, 2023 21:29
@brandyscarney brandyscarney force-pushed the FW-1808-wrap-update-screenshots branch from 753c24c to 162ff26 Compare June 15, 2023 19:47
@brandyscarney brandyscarney changed the base branch from main to FW-1808-wrap June 15, 2023 20:05
@brandyscarney brandyscarney changed the base branch from FW-1808-wrap to main June 20, 2023 21:12
@brandyscarney brandyscarney force-pushed the FW-1808-wrap-update-screenshots branch from 258f256 to b8320d8 Compare June 20, 2023 21:13
@brandyscarney brandyscarney changed the base branch from main to FW-1808-wrap June 20, 2023 21:33
@brandyscarney brandyscarney marked this pull request as ready for review June 20, 2023 21:42
@brandyscarney brandyscarney requested a review from a team as a code owner June 20, 2023 21:42
@brandyscarney brandyscarney requested review from averyjohnston and liamdebeasi and removed request for a team June 20, 2023 21:42
@github-actions github-actions bot added package: core @ionic/core package and removed package: core @ionic/core package labels Jun 27, 2023
@liamdebeasi liamdebeasi force-pushed the FW-1808-wrap-update-screenshots branch from 3369c22 to 486584e Compare June 27, 2023 21:13
@liamdebeasi liamdebeasi force-pushed the FW-1808-wrap-update-screenshots branch 2 times, most recently from fd6b913 to 8a19d41 Compare June 27, 2023 22:42
@liamdebeasi liamdebeasi force-pushed the FW-1808-wrap-update-screenshots branch from 2938516 to 7dae23a Compare June 27, 2023 23:00
@brandyscarney brandyscarney merged commit e18bc0f into FW-1808-wrap Jul 7, 2023
@brandyscarney brandyscarney deleted the FW-1808-wrap-update-screenshots branch July 7, 2023 17:52
brandyscarney added a commit that referenced this pull request Jul 7, 2023
)

Issue number: N/A - this does not completely resolve an issue but it
enables users to opt-in to having text wrap in a button by setting a
minimum height

---------

## What is the current behavior?
The current behavior when text is really long in a button is:
- Default buttons expand in width until part of the text (and button) is
off the screen and not in the visible viewport
- Block and full buttons horizontally align the text in the center and
overflow it on both sides (but the overflow is not visible so the text
is cut off at the beginning and end)

## What is the new behavior?
Allow the button height to increase when text wraps and add some padding
so that buttons with wrapped text still look nice. This does **NOT**
wrap the text in a button by default. That will be done in FW-4599.

- Removed `text-overflow: ellipsis` since this does not have any effect
- Changes `height` setting to `min-height` on all button types (small,
large, default) and buttons inside of an item, toolbar or list header
- Increases `padding-top` and `padding-bottom` on the buttons so that
overflowing buttons have padding around them
- Changes `.button-native` display property from `block` to `flex` in
order for anchor tags (`<ion-button href="#">` to align their text
vertically
- Sets `flex-shrink: 0` on slotted `start`/`end` elements to prevent
icons (and other elements) from shrinking to make room for the text
- Adds e2e test for button wrapping including the different types of
buttons that were changed by this PR
- Adds `ion-text-wrap` to the `ion-button` elements used in this test to
verify the height / padding changes are working as desired (to be
removed with FW-4599)
- Screenshot diffs are in the following PR:
#27561

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## Other information

This does **NOT** wrap the text in a button by default. It only enables
buttons to look nicer and auto adjust their height/padding when the text
is wrapping.

After internal discussion we decided that automatically making the text
wrap inside of a button may have undesired effects on existing apps. For
example, if someone has a button inside of a list header with a long
label, the button will now wrap if it has a space or dash in the text
content.

Developers should set `ion-text-wrap` on the `ion-button` to opt-in to
text wrapping in a button, and this will become the default as part of
FW-4599 (the next major release).

---------

Co-authored-by: ionitron <[email protected]>
Co-authored-by: Liam DeBeasi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants