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

Add overflow: hidden to .euiModal__flex #2164

Merged
merged 5 commits into from
Jul 29, 2019

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Jul 25, 2019

It was discovered in Kibana that even non-breaking words in code blocks inside of modals causes the content to overflow the body even though code blocks are setup to scroll.

https://d.pr/free/v/l0r1A1

This just hides any overflow from .euiModal__flex. Here's some tests in EUI docs:

Code blocks

Before

After

Also fixes #1762

Also "fixes" long titles #1766 #1384

... though it just really cuts it off and doesn't force it to break the word so I'm hesistent to call the above issues fixed and closed.

Checklist

  • This was checked in mobile
  • This was checked in IE11
  • [ ] This was checked in dark mode
  • [ ] Any props added have proper autodocs
  • [ ] Documentation examples were added
  • A changelog entry exists and is marked appropriately
  • [ ] This was checked for breaking changes and labeled appropriately
  • [ ] Jest tests were updated or added to match the most common scenarios
  • [ ] This was checked against keyboard-only and screenreader scenarios
  • [ ] This required updates to Framer X components

@cchaos
Copy link
Contributor Author

cchaos commented Jul 25, 2019

When I was checking in IE, there's always been this problem where IE wouldn't grow the width of the modal unless you explicitely gave it a width.

I found the answer which is just expanding the flex: 1 declaration to be the full flex: 1 1 auto.

So this fix is included in the PR as well.

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with this. I agree that it doesn't close the long title issues, but it's definitely less broken.

Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I gave up on verifying this in Kibana (I can't figure out the new linking; Kibana keeps crashing), but it does work as described in my local EUI docs. I'll check it again once Kibana is updated with this new EUI build.

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

Successfully merging this pull request may close these issues.

Modal with wide, non-breaking body text hides buttons
3 participants