-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
feat(chart): add feature flag that displays the data pane closed by default #21649
feat(chart): add feature flag that displays the data pane closed by default #21649
Conversation
bb377f0
to
e10482a
Compare
Codecov Report
@@ Coverage Diff @@
## master #21649 +/- ##
===========================================
- Coverage 66.82% 55.45% -11.38%
===========================================
Files 1799 1799
Lines 68867 68874 +7
Branches 7314 7318 +4
===========================================
- Hits 46022 38192 -7830
- Misses 20966 28798 +7832
- Partials 1879 1884 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
@Painyjames Thanks for the PR. It's a pretty useful feature for increasing the loading pace on Explore.
I found an issue when I tested this on my local environment. First, setting FF was enabled. Then, opened an existent Chart, the DataPanel was expected as closed for now. Next, opened the DataPanel and return to the Charts list. Finally, reopened the previous Chart but the DataPanel was white and seemed opened.
@zhaoyongjie I was not able to reproduce that issue locally, as it seems to be working fine. Could you spin up a testing instance with that flag on? |
I've replicated that locally and recorded the behaviour on this PR's branch: |
Hi @Painyjames I recorded the screen on the ephemeral environment. test.record.mov |
hi @zhaoyongjie , it appears collapsed when you enter into that chart, I think the carets might be odd in the new version though. I'll take a video shortly. |
cannot upload videos directly on github, but I've uploaded that here |
e10482a
to
3e82f2b
Compare
@zhaoyongjie you were right, I needed to make sure the padding was right on the |
/testenv up FEATURE_DATAPANEL_CLOSED_BY_DEFAULT=true |
@zhaoyongjie Ephemeral environment spinning up at http://18.236.146.0:8080. Credentials are |
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! I love this new feature since it reduces explore page loading time.
Ephemeral environment shutdown and build artifacts deleted. |
SUMMARY
Currently the Data section in the Chart screen is open or not depending on what the user has done previously in other charts (i.e. a flag is kept in the local storage that determines if theData section will need to be render or not depending of what the user's done in other chart screens).
Our team decided that we want that screen to have that section collapsed until the user clicks on the caret that opens it.
TESTING INSTRUCTIONS
After setting the new flag (DATAPANEL_CLOSED_BY_DEFAULT) to true, every time the chart screen is loaded it should have the Data section collapsed.
ADDITIONAL INFORMATION