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

[EuiDataGrid] Safari workaround for cell actions hover animation #6881

Merged
merged 3 commits into from
Jun 29, 2023

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented Jun 26, 2023

Summary

closes #6851

Problem

before

For some incredibly bizarre reason, Safari doesn't correctly update the flex width of the cell content (when rows are an undefined height/single flex row), which causes the action icons to overlap the content and makes the content less readable even when there is room for both the cell content and cell actions.

Solution

after

This workaround "animation" forces a rerender of the flex content width after the cell actions are done animating in. If the duration time is reduced, the fix becomes flaky/intermittent, occasionally working and occasionally not. No, I do not understand why 💀

I've filed a bug for Safari for this behavior here: https://bugs.webkit.org/show_bug.cgi?id=258539

QA

General checklist

  • Checked in Chrome, Safari, Edge, and Firefox
  • A changelog entry exists and is marked appropriately

@kibanamachine
Copy link

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

@cee-chen cee-chen requested a review from tkajtoch June 26, 2023 23:25
@cee-chen cee-chen marked this pull request as ready for review June 26, 2023 23:31
@kibanamachine
Copy link

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

Copy link
Member

@tkajtoch tkajtoch left a comment

Choose a reason for hiding this comment

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

Thanks for fixing it, it must've been a hard one to figure out!
Code looks good, tested on latest Chrome and Safari against PR env and latest release for comparison

@cee-chen
Copy link
Member Author

Thanks Tomasz! Yeah, that was a fun one. The fine folks at the webkit bugzilla repository got back to me and said this bug/behavior is fixed in Safari 17, so we should be able to just revert this workaround/commit wholesale once Safari is on v17. 🤞

@cee-chen cee-chen merged commit 10ca3e0 into elastic:main Jun 29, 2023
@cee-chen cee-chen deleted the 6851 branch June 29, 2023 21:40
1Copenut added a commit to elastic/kibana that referenced this pull request Jul 11, 2023
`[email protected]` ⏩ `83.1.0`

---

## [`83.1.0`](https://github.com/elastic/eui/tree/v83.1.0)

- Added `placeholder` prop to `EuiInlineEdit`
([#6883](elastic/eui#6883))
- Added `sparkles` glyph to `EuiIcon`
([#6898](elastic/eui#6898))

**Bug fixes**

- Fixed Safari-only bug for single-line row `EuiDataGrid`s, where cell
actions on hover would overlap instead of pushing content to the left
([#6881](elastic/eui#6881))
- Fixed `EuiButton` not correctly merging in passed `className`s with
its base `.euiButton` class
([#6887](elastic/eui#6887))
- Fixed `EuiIcon` not correctly passing the `style` prop custom `img`
icons ([#6888](elastic/eui#6888))
- Fixed multiple components with child props (e.g. `buttonProps`,
`iconProps`, etc.) unsetting EUI's Emotion styling if custom `css` was
passed to the child props object
([#6896](elastic/eui#6896))

**CSS-in-JS conversions**

- Converted `EuiHeader` and `EuiHeaderLogo` to Emotion
([#6878](elastic/eui#6878))
- Removed Sass variables `$euiHeaderDarkBackgroundColor`,
`$euiHeaderBorderColor`, and `$euiHeaderBreadcrumbColor`
([#6878](elastic/eui#6878))
- Removed Sass mixin `@euiHeaderDarkTheme`
([#6878](elastic/eui#6878))
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.

[DataGrid] Table custom actions won't push cell content on the left on hover in Safari
3 participants