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

Remove IE flex and display CSS fixes & fallbacks #6161

Merged
merged 12 commits into from
Aug 24, 2022

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented Aug 23, 2022

Summary

Follow up to #6154. While #6154 is a set of fairly safe IE fallback removals, this PR is slightly more risky (particularly dadedf1), and is a separate PR for more granular testing and potentially easier rollback if needed.

Components to QA

Checklist

  • Checked in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • Checked for breaking changes and labeled appropriately
  • A changelog entry exists and is marked appropriately

- [ ] Props have proper autodocs and playground toggles
- [ ] Added documentation
- [ ] Checked Code Sandbox works for any docs examples
- [ ] Added or updated jest and cypress tests
- [ ] Checked for accessibility including keyboard-only and screenreader modes
- [ ] Updated the Figma library counterpart

- truncation and display still appears to all work as before without issue
- flex-grow is still necessary for icons to be pushed to the right
- looks to have been missed in the Emotion conversion
- likely not necessary to test as EuiPageContent is deprecated
- not sure if this causes unexpected issues :/
@cee-chen cee-chen requested a review from miukimiu August 23, 2022 17:54
@cee-chen
Copy link
Member Author

@miukimiu tagging you in both PRs as well for the CSS changes, and also because you're amazing at QA 🙇 No rush on these PRs, take your time!

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6161/

Copy link
Contributor

@miukimiu miukimiu left a comment

Choose a reason for hiding this comment

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

Thanks, @constancecchen.
I tested all the components in Chrome, Safari, and Firefox:

  • EuiModal
  • EuiBadge
  • EuiCard
  • EuiForm
  • EuiListGroup
  • EuiBasicTable mobile cells
  • EuiFlexGroup / EuiFlexItem (Let's keep the flex-grow: 1;)
  • EuiPageContent not worth testing as it's a deprecated component

LGTM! 🎉 Let's just keep that .euiFlexGroup { flex-grow: 1;}.

@cee-chen
Copy link
Member Author

Thanks a million Elizabet!!

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6161/

@cee-chen cee-chen merged commit ae4d489 into elastic:main Aug 24, 2022
@cee-chen cee-chen deleted the remove-ie-css-flex branch August 24, 2022 18:49
@miukimiu miukimiu mentioned this pull request Oct 27, 2022
26 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants