-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
fix(chart): scrollbar keep flusing on and off #23778
fix(chart): scrollbar keep flusing on and off #23778
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! This might actually close out a lot of GitHub Issues if we're lucky! THANK YOU!
I fixed the Frontend CI error here. |
a75dc48
to
fee6195
Compare
Codecov Report
@@ Coverage Diff @@
## master #23778 +/- ##
==========================================
+ Coverage 67.91% 68.02% +0.11%
==========================================
Files 1936 1937 +1
Lines 74929 74937 +8
Branches 8141 8143 +2
==========================================
+ Hits 50886 50974 +88
+ Misses 21955 21874 -81
- Partials 2088 2089 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 10 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@michael-s-molina the CI error came from here and I already fixed with replacing mock. |
b8e7195
to
4695894
Compare
6cbc47d
to
76e8d21
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This reverts commit 839bc08.
Hi @justinpark we're seeing some regression with this PR when running reports, or viewing a chart in standalone mode. Do you have time to put up a quick fix for this issue? Let me know if you need any help reproducing. |
This reverts commit 839bc08.
SUMMARY
This bug introduced by #20282
After resize target element has scrollbars, the chart keeps flushing scrollbars on and oft.
It because keeps re-calculate the element's size due to the content size is different with/without scrollbars.
This commit replaces the resize observe element with the parent element (which has no scrollbars) and then calculate the chartPanel element from onResize handler.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
After:
after--chart-ref-observe.mov
Before:
before--chart-ref-observe.mov
TESTING INSTRUCTIONS
Open a chart explore
Resize window width smaller
check the chart scroll
ADDITIONAL INFORMATION