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

Modals and calllouts need better text-wrapping #1384

Closed
cjcenizal opened this issue Dec 18, 2018 · 9 comments
Closed

Modals and calllouts need better text-wrapping #1384

cjcenizal opened this issue Dec 18, 2018 · 9 comments
Labels

Comments

@cjcenizal
Copy link
Contributor

Per elastic/kibana#27365 and elastic/kibana#27368 the text content of these components overflows on long lines:

screen shot 2018-12-17 at 10 12 26 pm

screen shot 2018-12-17 at 10 59 58 pm

@snide
Copy link
Contributor

snide commented Dec 19, 2018

I'd use the CSS utility classes for this.

@snide
Copy link
Contributor

snide commented Dec 19, 2018

@cjcenizal
Copy link
Contributor Author

Can we apply these classes to the implementation of these components? Otherwise consumers will need to use those classes for every instance in which a modal or callout contains dynamic content.

@snide
Copy link
Contributor

snide commented Dec 19, 2018

The problem is these are sometimes rather than all the time implementations so I think it's better to apply a selector the times you need it then provide props everywhere that do the same thing through a Boolean. Right now the text is just acting like normal text, which is expected until it isn't.

The counter pattern I'm trying to avoid is table cells which have all these extra props and complexity for a simple concept. The utility classes are good for this kind of thing. They work everywhere and give consumers the freedom manipulate the core content however they want, regardless of the component.

@snide
Copy link
Contributor

snide commented Dec 19, 2018

Put another way. I could see this same request repeated once a month for any component involving text.

@cjcenizal
Copy link
Contributor Author

cjcenizal commented Dec 19, 2018

The problem is these are sometimes rather than all the time implementations

I just realized that with localization on the horizon, even static text is subject to change. For example, a UI could look great in English but then look broken in German. Because of this, we'll need to "long-text-proof" all UIs -- or at the very least begin with the components which are narrow and more likely to break, like toasts, tooltips, and the smaller modal.

Can we consider "long-text-proofing" as a primary concern in EUI, the same way we do with accessibility? Maybe even add it to the PR checklist? And if it's baked into the component then we won't need to worry about adding props or asking consumers to apply utility classes.

@chandlerprall
Copy link
Contributor

chandlerprall commented Dec 19, 2018

IMO (and only my opinion), I think our design and guidelines need to determine what is the expected content & use of an onscreen area and provide the best CSS (and code) defaults. I think we've done that well to date, but in English only. The i18n efforts means we need to re-adjust some baselines, but not necessarily the approach or consideration. In this specific case (and IMO) the dialog title is not expected to overflow - the guideline is to keep it short, and the dialog body is designed to expand. Perhaps there is a failure in not better documenting those expectations in our guidelines?

It's my understanding that internationalized text will be spot-checked in place (at least in the beginning) to help detect these areas.

@snide
Copy link
Contributor

snide commented Dec 20, 2018

I'll set up a meeting post-holidays to go over this in more detail and it should be considered with the i18n stuff. There are adequate tools to address this in the short term so we have some time to ponder.

Let's just be cautious of using a sledgehammer for text. Word breaks aren't fun to see so it's good to minimize them from busting perfectly reasonable content. The overflow-wrap stuff should help.

@snide
Copy link
Contributor

snide commented Sep 12, 2019

This was fixed with #2164

@snide snide closed this as completed Sep 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants