fix(item): elements should wrap and label should not collapse when text size increases - #28146
Conversation
Issue number: N/A
---------
## What is the current behavior?
The datetime button does not support dynamic type.
## What is the new behavior?
Adds support for dynamic type by converting `font-size` to use `rem`
units and adding an ellipsis to text when the size is too large.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
## Other information
Dynamic type can be enabled with the following CSS:
```css
html {
--ion-dynamic-type: var(--ion-default-dynamic-font);
}
```
Adds support for dynamic type by converting `font-size` to use `rem` units and setting a `max-size` which matches native iOS.
- Adds support for dynamic type by converting `font-size` to use `rem` units. - Updates the button width/height for icon only buttons in `md` to use `rem` units to scale the icon larger - Adds test for scaling the font which checks default buttons, clear buttons (because they use a different `font-size`), buttons with an icon, and a button containing only an icon
Issue number: N/A --------- <!-- 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. --> The chip component does not support dynamic type. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> The chip component supports dynamic type, has a dynamic height, has min and max font sizes in `ios` mode, and has a screenshot test. ### md mode Small md mode: <img width="1368" alt="md-small" src="https://github.com/ionic-team/ionic-framework/assets/14926794/4466c1b6-6d01-4ec8-8de9-0eb71fa45223"> Medium md mode: <img width="1371" alt="md-medium" src="https://github.com/ionic-team/ionic-framework/assets/14926794/c51c324e-a3b7-486b-9f68-84b0e4146570"> Large md mode: <img width="1369" alt="md-large" src="https://github.com/ionic-team/ionic-framework/assets/14926794/166e1229-96b7-4270-9ef2-4c708776691e"> ### ios mode Small ios mode: <img width="1371" alt="ios-small" src="https://github.com/ionic-team/ionic-framework/assets/14926794/de240058-5fe5-4387-8481-d695950853db"> Medium ios mode: <img width="1371" alt="ios-medium" src="https://github.com/ionic-team/ionic-framework/assets/14926794/c63db4be-e813-4917-a525-02ec456849ea"> Large ios mode: <img width="1370" alt="ios-large" src="https://github.com/ionic-team/ionic-framework/assets/14926794/3479f619-82c8-4645-85f7-2725ea43de00"> ## 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. --> This also fixes an existing bug that the contents of a chip could get smooshed if the chip doesn't have enough horizontal space for its contents. ### Before <img width="253" alt="rStll" src="https://github.com/ionic-team/ionic-framework/assets/14926794/9a8f4c30-ceef-4fe1-a600-38b2f01c4e68"> ### After <img width="253" alt="Screenshot 2023-08-02 at 5 00 39 PM" src="https://github.com/ionic-team/ionic-framework/assets/14926794/e82e17c3-91e3-4089-a57b-16c7d7b386fb"> --------- Co-authored-by: ionitron <hi@ionicframework.com> Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
Adds support for dynamic type by converting `font-size` to use `rem` units and setting a max `font-size` which matches native iOS.
Adds support for dynamic type by converting `font-size` to use `rem` units and setting a min `font-size` which matches native iOS.
Adds support for dynamic type by converting `font-size` to use `rem` units.
Adds support for dynamic type by converting `font-size` to use `rem` units.
Adds support for dynamic type by converting `font-size` to use `rem` units and adds a min and max `font-size` for `ios`.
Issue number: N/A --------- <!-- 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. --> The dynamic type feature branch screenshots are out of sync with the updated state from changes on `main`. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Regenerates update screenshot diffs for the dynamic type feature branch with the latest changes from `main` ## 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>
chore: sync with main
Adds support for dynamic type by converting `font-size` to use `rem` units. Upgrades the `sass` version to `1.33.0` to support this change.
Adds support for dynamic type by converting `font-size` to use `rem` units and adds a min and max `font-size` for `ios`.
Issue number: N/A --------- <!-- 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. --> ion-textarea does not support dynamic type ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - ion-textarea does support dynamic type ## 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. --> Mostly, dynamic type for textarea was already supported. Only the bottom-content had a static size. All other font sizes were all already relative. Note: Textareas do not currently look correct when they are in an item, because #27989 has not yet merged. Once that PR is merged, the font sizes within textareas will be relative to the item sizes, and both will scale appropriately. If preferred, I can base this PR off branch FW-4432 instead. --------- Co-authored-by: ionitron <hi@ionicframework.com>
Co-authored-by: ionitron <hi@ionicframework.com> Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
liamdebeasi
left a comment
There was a problem hiding this comment.
One comment, but good to go once that's addressed.
|
Not sure I agree with this not being a breaking change, had to go and put in ion-text-nowrap in a lot of places in my app. Should be mentioned in the notes. |
|
I've updated the release notes, but I recommend allowing text to wrap by default. The previous behavior was broken from an accessibility perspective, so forcing text to not wrap can degrade the experience for many users. We don't consider this a breaking change because the previous experience was broken for many users. |
We got some feedback in #28146 (comment) that it would be helpful to note the new item wrapping behavior in v7.6. I've updated the Github release, and this PR updates the Changelog file.
Specifically in places where I also had an ion-input with the label, a label that was previously displaying fine, was now splitting into multiple lines even when there is plenty of room for the label. |
|
I built a lot of custom CSS upon the ion-item for our specific needs, and this minor update was definitely breaking a lot for us 😅 Maybe I shouldn't have used the ion-items in these custom UI's Added a video to show some of the context where this is not desired. Is there a way to opt-out? I cannot find a way to overwrite because the changes are not exposed. Screen.Recording.2023-12-13.at.15.59.33.mov |
|
You can set |
This utility does nothing to undo the changes to flex-wrap, flex-grow and shrink properties. It looks like I can't overwrite them either, because these elements seem to be unexposed. Is there any other workaround, besides rebuilding everything without using ion-item or wrapping it in a container? Although an improvement, it would be great to be able to opt-out from this wrapping behaviour, as there are some scenarios where it's not desired. Maybe a custom property |
|
I also had some elements where we used ion-item as a base container for custom elements that I had to rewrite. Still do not agree this is not a breaking change. Found no workaround |
|
Try setting Previously the flex-basis was The Of course, if you get down to smaller widths such as 100px, the elements may still wrap/get truncated. However, I'd argue that not wrapping content at that small of a width makes the content hard to understand since most text will be not readable. Here's a demo: https://codepen.io/liamdebeasi/pen/mdvZdjg |
|
Hi everyone, The team is having discussions into revising part of the text wrapping behavior. In particular, we are considering not having the end/start slot content wrap to the next line. In other words, only the text in the default slot would wrap within its own container. We are having this discussion based on some new information we've found related to how iOS handles wrapping. We're tracking this in #28769. |
|
Is there anyway we can set the item-legacy class globally on all instances of ion-item and ion-item-divider without having to apply it to each one individually? Maybe add a property to IonicConfig for this so we can say: This recent change of wrapping ion-label by default does in a way break the UI of my app. |
Issue number: resolves #28769 --------- <!-- 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 #28146, we allowed text wrapping inside of `ion-item` for accessibility purposes. One of the behaviors we added was to allow start, default, and end slotted containers to wrap to the next line to align with the iOS spec. However, this decision was based on an incorrect assumption. The following screenshot shows the Settings app on iOS: | default scale | 310% scale | | - | - | | <img width="1170" alt="Frame 4" src="https://github.com/ionic-team/ionic-framework/assets/2721089/462ef153-a060-41c8-9a00-f0aad17839be"> | <img width="1170" alt="Frame 5" src="https://github.com/ionic-team/ionic-framework/assets/2721089/f047f880-7b80-4710-939b-96da075fbbf9"> | At the default scale, the blue icon is in the iOS equivalent of the "start" slot, "Bluetooth" is in the default slot, and "On" is in the "end" slot. We incorrectly assumed the same markup was true when scaling the text up. However, at 310% scale the icon, "Bluetooth" text, and "On" text all become part of the default slot in a single container that wraps. You can tell because the bottom border runs underneath the blue icon at 310% whereas it does not at the default scale. This allows the text to wrap underneath the blue icon. When we originally implemented #28146 we thought that this meant the start, default, and end slot containers should wrap to the next line. I further validated this behavior by creating an app with Swift UI. I created a list of items where each item has the native equivalent of a checkbox in the `start` slot and multiple `ion-labels` in the default slot of the item: | Default Scale | 310% Scale | | - | - | |  |  | The content within each label wraps within the container, but the containers themselves never wrap to the next line. Demo code: ```swift import SwiftUI struct ContentView: View { struct Item: Identifiable, Hashable { let id = UUID() } private var items = [ Item(), Item(), Item(), Item(), Item() ] @State private var multiSelection = Set<UUID>() var body: some View { NavigationView { List(items, selection: $multiSelection) {_ in HStack { Text("Column 1 with really long text") Text("Column 2 with really long text") Text("Column 3 with really long text") Text("Column 4 with really long text") } } .toolbar { EditButton() } } Text("\(multiSelection.count) selections") } } #Preview { ContentView() } ``` ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - This PR removes the ability for the start, default, and end slot containers to wrap to the next line. This behavior aligns with pre-v7.6.0 behaviors. The containers inside of the default slot will not wrap to the next line. However, content within each container (such as text within an `ion-label`) will continue to wrap to meet the team's accessibility requirements. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> Dev build: `7.6.5-dev.11704916749.1e64a3a7` --------- Co-authored-by: ionitron <hi@ionicframework.com>
This will resolve the following internal tickets:
What is the current behavior?
There are a few problems with how labels inside of items behave. Labels inside of items do not wrap their text, instead they truncate with an ellipsis. The label will continue to show an ellipsis while shrinking if there is wide content next to it, until eventually it completely disappears. This is especially problematic for dynamic type, when the font size can be increased up to
310%onios. Below are some screenshots of how Ionic behaves when increasing the font size with anion-labelandion-notein anion-item. You'll notice that theion-label(Wi-Fi) completely disappears at310%.100%145%310%Here is how the same item looks on native iOS at
310%:What is the new behavior?
A lot of changes were made in order to support wrapping components inside of an item. Since a lot of the code in
ion-labelandion-itemis used throughout multiple components, several changes were wrapped into this PR.white-space: nowrap) and allow it to grow.legacyclass was added to item (.item-legacy) by all of the components that havelegacyas a property. This class was targeted in the CSS to undo the wrapping changes for legacy items, otherwise it breaks a lot of tests: 6ec8bf4a) Tests with legacy inputs were updated to include the
legacyattribute. Without it the UI will break for several examples: a8a34ceitem/test/texttest was removed. It was almost exactly a duplicate ofitem/test/basic: eb79794widthtoauto, undoing where I set the width tomin-contentelsewhere, in order to prevent it from wrapping all of the time. I also added a comment linking to a JIRA ticket to remove the Item because of the issues it causes by being used: 88106d8a11yfor Note to ensure that the Label next to a Note doesn't disappear: 1f23ffcion-text-wrapand/or addion-text-nowrapto prevent screenshot diffs. For exampleskeleton-textclass up to item (asitem-skeleton-text) in order to addoverflow: hiddento the label for skeleton text wrappers: 077fb08 - this also addsoverflow: hiddenfor legacy labels to keep the ellipsis but the modern items do not need itDoes this introduce a breaking change?
Other information
Screenshots for this branch are separated into the following PR: #28278