[EuiDataGrid] Fix grid height when exiting full screen mode #5580
Conversation
- so grid height restores correctly after leaving full-screen mode
cee-chen
added
the
skip-changelog
| return IS_JEST_ENVIRONMENT | ||
| ? { |
There was a problem hiding this comment.
@chandlerprall Raising this because we just chatted about IS_JEST_ENVIRONMENT - is there any realistic way of unit testing this entire helper in Jest without running into this conditional? 🤔
There was a problem hiding this comment.
Only thing that comes to mind is to rewrite relevant tests in Cypress, but ship a mockenv version for downstream test environments. That gets weird with the other two functions in this file, and we'd probably want to split this function out into its own file to mock specifically. So, no, I don't have any great ideas
There was a problem hiding this comment.
Ooh, I totally forgot about .testenv files - I believe I can import the actual functions for the other 2 fns that don't have mocks and export only a fake fn for the one we want to target. Perfect! I'll look into that as a follow-up PR
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5580/ |
|
jenkins test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5580/ |
chandlerprall
left a comment
chandlerprall
left a comment
There was a problem hiding this comment.
Changes LGTM; I tested on the CI branch and everything acts right, but I also cannot replicate this bug from main so we should probably get another tester too.
There was a problem hiding this comment.
LGTM! 🎉
@chandlerprall, I tested the main branch locally and this issue happens to me in Chrome/Firefox/Safari.
Tested this PR in Chrome/Firefox/Safari and the issue is no longer happening!
…5580) * Track full screen grid height separately - so grid height restores correctly after leaving full-screen mode * Fix nested ternary eslint warning
4 participants
Summary
Super huge kudos to @miukimiu for catching this bug before it shipped! I introduced it in #5557 🙈
Repro steps
Before
After
Checklist
- [ ] Added or updated jest and cypress testsIS_JEST_ENVIRONMENTconditional- [ ] A changelog entry exists and is marked appropriately