eui icon indicating copy to clipboard operation
eui copied to clipboard

:bug: trying to fix data grid bug, binded with gridStyles

Open pamellix opened this issue 1 year ago • 2 comments

Summary

I think that the bug can be fixed if we add a gridStyles to a deps array for useUpdateEffect for grid styles. Analysing the code I spotted that changes are commited if function onChange in gridStyles was called, that's why setDensity in reset button did not changed the style. So, I think that if some changes were made in gridStyles, then onChange should be called.

General checklist

  • Browser QA
    • [ ] Checked in both light and dark modes
    • [ ] Checked in mobile
    • [ ] Checked in Chrome, Safari, Edge, and Firefox
    • [ ] Checked for accessibility including keyboard-only and screenreader modes

pamellix avatar Aug 15 '24 00:08 pamellix

❌ Author of the following commits did not sign a Contributor Agreement: 17e37c0c834a148e917ac08171426bc41024f885, e64f31b1621771898a74d7fd3c62753968302bbf

Please, read and sign the above mentioned agreement if you want to contribute to this project

👋 Since this is a community submitted pull request, a Buildkite build has not been started automatically. Would an Elastic organization member please verify the contents of this pull request and kick off a build manually?

github-actions[bot] avatar Aug 15 '24 00:08 github-actions[bot]

This doesn't solve https://github.com/elastic/eui/issues/7962 unfortunately. There's a lot more at play here in this logic which involves figuring out which updates need to take precedence (dev styles vs user styles). If we're going to start listening for updates to gridStyles from developers, we also need to not fire onChange since those should only fire on user style changes.

I'm going to go ahead and close this PR - it's likely the EUI team will pick up #7962, #7951, and #5593 simultaneously later.

cee-chen avatar Aug 30 '24 03:08 cee-chen