-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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: multiple results pane on explore and dashboard #20277
feat: multiple results pane on explore and dashboard #20277
Conversation
db59f48
to
c209bc0
Compare
Codecov Report
@@ Coverage Diff @@
## master #20277 +/- ##
==========================================
+ Coverage 66.61% 66.67% +0.05%
==========================================
Files 1733 1737 +4
Lines 64953 64973 +20
Branches 6858 6869 +11
==========================================
+ Hits 43268 43319 +51
+ Misses 19929 19905 -24
+ Partials 1756 1749 -7
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
83d195d
to
fe3330b
Compare
/testenv up |
@zhaoyongjie Ephemeral environment spinning up at http://34.221.119.163:8080. Credentials are |
fe3330b
to
001e8c0
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.
Great feature! I only have 1 comment about the labels - I don't think that the results of second query should be labeled as "Results 1". I think that for multiple queries we should have either: Results / Results 2
or Results 1 / Results 2
or Results A / Results B
(since the queries are labeled as A
and B
in control panel). In the charts that have only 1 query we should keep the label Results
.
Names are always a difficult thing. "Results 2" make sense. if have any good ideas, I will change them. |
@kasiazjc WDYT? |
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.
Tested, works great except 1 minor thing. In Explore, when I change time column formatting from formatted to original, the change is applied in all tabs (results, results 2, samples). When I do the same thing on Dashboard, the change is applied only in 1 tab for that column.
Approving since it's a minor, non-blocking problem, but we may want to fix it before merging if it's quick and easy to fix
Ephemeral environment shutdown and build artifacts deleted. |
@kgabryje I intend to use Context to refactor the time columns switcher since it's hard to control different results pane in Dashboard. Could I implement it in the separate PR? |
I merged first, then fix it in the separate PR. |
SUMMARY
Currently, the Superset QueryContext contains multiple queries but the south pane in the Explore only supported single results pane. this PR intends to support multiple results pane in the Explore page and Dashboard.
This PR also add fully unit test for SamplePane and ResultsPane.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
multiple.results.pane.mov
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION