Skip to content
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

docs: item does not account for new text wrapping behavior #3313

Closed
3 tasks done
lincolnthree opened this issue Dec 11, 2023 · 18 comments · Fixed by #3314
Closed
3 tasks done

docs: item does not account for new text wrapping behavior #3313

lincolnthree opened this issue Dec 11, 2023 · 18 comments · Fixed by #3314
Labels
triage New issues

Comments

@lincolnthree
Copy link

Prerequisites

Ionic Framework Version

v7.x

Current Behavior

Screenshot 2023-12-11 at 2 35 23 PM

Ellipsis is no longer working as of Ionic 7.6.0

Expected Behavior

Text should wrap/ellipsis when labels get too long in ion-item components unless ion-text-wrap styles are applied.

Steps to Reproduce

Visit the documentation for ion-item. See the error in the basic usage section:

https://ionicframework.com/docs/api/item#basic-usage

Code Reproduction URL

https://ionicframework.com/docs/api/item#basic-usage

Ionic Info

Ionic:

Ionic CLI : 7.1.1 (/Users/lincoln/.nvm/versions/node/v16.14.0/lib/node_modules/@ionic/cli)
Ionic Framework : @ionic/angular 7.6.0
@angular-devkit/build-angular : 17.0.6
@angular-devkit/schematics : 17.0.6
@angular/cli : 17.0.6
@ionic/angular-toolkit : 10.0.0

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage New issues label Dec 11, 2023
@liamdebeasi
Copy link
Contributor

Thanks! We added text wrapping to ion-item by default for accessibility but we forgot to update the documentation. I'll update that now.

@lincolnthree
Copy link
Author

This also causes breaking block-level element wrapping to occur if you try to actually remove text-wrapping manually:

image

@lincolnthree
Copy link
Author

lincolnthree commented Dec 11, 2023

Thanks! We added text wrapping to ion-item by default for accessibility but we forgot to update the documentation. I'll update that now.

Thanks. What is the recommended method to enable ellipsis going forward? Note my previous message. We depend on the ellipsis functionality in a lot of places and attempting to upgrade to 7.6.0 breaks most of our screens.

@liamdebeasi liamdebeasi transferred this issue from ionic-team/ionic-framework Dec 11, 2023
@liamdebeasi
Copy link
Contributor

You can use .ion-text-nowrap to prevent wrapping. However, I recommend allowing wrapping as truncating often times makes that content inaccessible to users.

@liamdebeasi
Copy link
Contributor

This also causes breaking block-level element wrapping to occur if you try to actually remove text-wrapping manually:

The browser is working correctly here. pre cause lines to only be broken at newline characters and <br /> elements. Your code makes it so the text in ion-label does not wrap. However, the ion-item still allows wrapping. If the ion-item did not wrap, then the ion-button would be pushed out of view. As a result, the button wraps to the next line so it is still visible.

@lincolnthree
Copy link
Author

lincolnthree commented Dec 11, 2023

Ideally it would be possible [just to wrap all text], but because virtual-scrolling makes variable-height items difficult (or impossible, without a good library available), we have to truncate in any place where VS is used.

Or when a side-panel is opened and we don't want to cause a ton of layout shift:

image

(layout needs to stay where it is when side panel is opened...)

image

@liamdebeasi
Copy link
Contributor

Ideally it would be possible, but because virtual-scrolling makes variable-height items difficult (or impossible, without a good library available), we have to truncate in any place where VS is used.

Ah I see, that's too bad. In that case I'd recommend setting .ion-text-nowrap on ion-item. Here is a demo: https://ionic-docs-git-item-wrapping-ionic1.vercel.app/docs/api/item#basic-usage

@liamdebeasi liamdebeasi changed the title bug: ion-item 'ellipsis' behavior is broken -- all text wraps docs: item does not account for new text wrapping behavior Dec 11, 2023
@lincolnthree
Copy link
Author

lincolnthree commented Dec 11, 2023

Thanks, this is helpful.

One interesting thing... With ion-text-nowrap the ellipsis functionality returns, but the block-level elements in the item still break to the next line:

image

If I set the width of the label to 0px, it works, even though the label is definitely not rendered at 0px wide, lol:

image

@liamdebeasi
Copy link
Contributor

Sorry, I'm not seeing the difference in behavior. Do you have a minimal repro?

@lincolnthree
Copy link
Author

https://stackblitz.com/edit/angular-zx6jyk?file=src%2Fapp%2Fexample.component.html

<ion-item>
  <ion-label class="ion-text-nowrap">
    Multi-line text that should ellipsis when it is too long to fit on one line. Lorem ipsum dolor sit amet,
    consectetur adipiscing elit.
  </ion-label>
  <ion-label slot=end>Foo</ion-label>
</ion-item>

@lincolnthree
Copy link
Author

lincolnthree commented Dec 11, 2023

Sorry, easier to see with a button... forgive my copy-pasta:
Screenshot 2023-12-11 at 3 15 06 PM

@lincolnthree
Copy link
Author

lincolnthree commented Dec 11, 2023

@liamdebeasi Did you see my reproduction of the layout breaking issue? #3313 (comment)

Is this expected?

Prior to 7.6.0, Ionic would dynamically size the label/inner content of the item to fit between the 'start' and 'end' slot content.

@liamdebeasi
Copy link
Contributor

Yes I did. I am discussing with the team regarding this.

Sorry, the issue got closed automatically when #3314 merged since it was linked in the PR description.

@lincolnthree
Copy link
Author

Yes I did. I am discussing with the team regarding this.

Sorry, the issue got closed automatically when #3314 merged since it was linked in the PR description.

Thanks, gotcha! No worries. I just wanted to make sure it didn't get lost. Looking forward to hearing what you figure out.

@liamdebeasi
Copy link
Contributor

I spoke with the team, and the width: 0 usage you set on ion-label is correct. The relevant change that caused this difference is in https://github.com/ionic-team/ionic-framework/blob/fc88613fefa019a3b695a2c6e10c85cd3ce79ae8/core/src/components/item/item.scss#L444.

We changed this from flex: 1 0 0 to flex: 1 0 auto. The last value, the flex-basis, defines the initial main size of the flex item. Previously, the initial main size was 0, and now it's auto. If we did not make this change then using multiple text elements in ion-item would cause some of the text to collapse to 0 (such as a label with a note).

The reason why your width: 0 CSS works is it effectively makes the flex-basis 0 again, since the auto keyword causes flex-basis to use the value of width.

@lincolnthree
Copy link
Author

lincolnthree commented Dec 18, 2023

Thanks @liamdebeasi -- Interesting. I'm not a flex-box expert.

What is the best way to undo this change in our CSS? Even with width: 0 we are getting all kinds of layout inconsistencies on pretty much every page of the app. Even our login form. It will mean a full style review and changes to every component and screen.

EDIT: Actually I think the issue may be that we need to know the best way to target the width: 0 style. Right now it's only being applied to ion-label, but there are other non-label elements in our items.

@liamdebeasi
Copy link
Contributor

Just a heads up, 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 ionic-team/ionic-framework#28769. If we end up making this change this should address the issue you noted in #3313 (comment).

@lincolnthree
Copy link
Author

lincolnthree commented Jan 8, 2024

Thanks Liam. Glad to hear further discussion is happening. To be honest, this issue has blocked us from updating to the latest Ionic versions. Even after auditing every screen, we can't come up with a global ruleset to fix the issues caused by this change. It's forcing us to add custom styles to every element. We're very close to forking ion-item entirely. While it looks like we could add the .item-legacy style to every item, that won't resolve the issue once legacy items go away entirely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage New issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants