-
Notifications
You must be signed in to change notification settings - Fork 14k
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
refactor: decouple DataTablesPane #20109
Conversation
d28d8a5
to
cde8b29
Compare
Codecov Report
@@ Coverage Diff @@
## master #20109 +/- ##
=======================================
Coverage 66.43% 66.43%
=======================================
Files 1722 1722
Lines 64640 64644 +4
Branches 6822 6822
=======================================
+ Hits 42944 42948 +4
Misses 19962 19962
Partials 1734 1734
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
864cc1e
to
1f82ad6
Compare
superset-frontend/src/explore/components/DataTablesPane/useGetResultsOrSamples.ts
Outdated
Show resolved
Hide resolved
7281a79
to
a69bc35
Compare
d42b55b
to
0340ab0
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 and tested to work well. I tried my best to compare some of the removed/added files, but it was difficult to get a good grasp of which functionality had changed, but everything looks sensible.
const { colnames, coltypes } = json.result[0]; | ||
// Only displaying the first query is currently supported |
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.
Idea for follow-up: Now it should be fairly simple to add support for displaying the other queries data, too. We could have a dropdown next to the search field where you could choose the query if there are more than one available.
SUMMARY
In order to show the
results pane
in the Dashboard(separate PR), this PR decouples DataTablesPane and fix some bugs from the original implementation.Results Pane
andChart Pane
parallel sending queries.Results Pane
if it's an invalid query.Results Pane
andsamples Pane
when switching datasetBEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
force-query Samples Pane
force-query.in.samples.pane.mov
trigger re-query Samples Pane after editing the dataset
re-query.samples.pane.mov
parallel sending query
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION