fix(alert): radio and checkbox labels wrap to next line - #27898
Merged
Conversation
|
|
liamdebeasi
commented
Jul 31, 2023
|
|
||
| .alert-tappable { | ||
| height: $alert-ios-tappable-height; | ||
| min-height: $alert-ios-tappable-height; |
Contributor
Author
There was a problem hiding this comment.
The ticket notes that we should set height: auto, but I found that created some undesired diffs on radios/checkboxes with short text. I found that min-height gets us to the same desired result without the additional diffs for short text.
liamdebeasi
commented
Jul 31, 2023
| width: 100%; | ||
| height: 100%; | ||
|
|
||
| min-height: inherit; |
Contributor
Author
There was a problem hiding this comment.
Without this the button inner may have visual diffs since the height is no longer fixed at 48px
liamdebeasi
commented
Jul 31, 2023
| appearance: none; | ||
|
|
||
| contain: strict; | ||
| contain: content; |
Contributor
Author
There was a problem hiding this comment.
Without this, each radio/checkbox will still be sized at 48px. This change allows the height of the element to increase as the text wraps.
liamdebeasi
marked this pull request as ready for review
July 31, 2023 17:21
brandyscarney
left a comment
brandyscarney
left a comment
Member
There was a problem hiding this comment.
Code looks good. I am wondering if we shouldn't separate out the wrapping example. Do we have an existing screenshot for checkbox/radio in an alert without wrapping?
brandyscarney
approved these changes
Aug 1, 2023
brandyscarney
left a comment
brandyscarney
left a comment
Member
There was a problem hiding this comment.
Looks good to me! The changes to min-height and contain make sense.
liamdebeasi
requested review from
averyrousseau and
sean-perkins
as code owners
August 1, 2023 14:54
github-actions
Bot
added
package: angular
github-actions
Bot
removed
package: angular
2 tasks
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Oct 25, 2023
Issue number: resolves #28406 --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> As part of #27898 we updated the radio and checkbox labels to wrap to the next line instead of truncate. However, we did not consider long words. As a result, long words run outside of the container. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - The radio and checkbox labels now break on words too in addition to white space characters. ## 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 <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> --------- Co-authored-by: ionitron <hi@ionicframework.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue number: resolves #17269
What is the current behavior?
Long radio and checkbox labels truncate with ellipsis instead of wrapping to the next line. This makes the label hard to understand because users cannot read the entire label.
What is the new behavior?
Note: Some of the screenshots may show wider alerts than before. This is the correct behavior. When rendering text, the browser will try to render as much text on a single line as it can. Since our alerts can expand in width, the alert wrapper will expand in width (up to the max width) to accommodate this text. Once the the wrapper is at the max width, text will then wrap to the next line.
For example, you may notice the MD alert increasing to 280px in width:
ionic-framework/core/src/components/alert/alert.md.vars.scss
Line 11 in b8553d8
There should be no visual diff for alerts with text that would not normally wrap to the next line.
This was not happening before because we did not allow text to wrap to the next line.
Does this introduce a breaking change?
Other information