Skip to content
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: Refactor QueryTable to use react-table #11216

Merged
merged 10 commits into from
Oct 19, 2020

Conversation

kgabryje
Copy link
Member

@kgabryje kgabryje commented Oct 9, 2020

SUMMARY

Re-reverts #10981, which was reverted due to the issue #11096. Uses custom TableView component based on react-table library.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

Sorry, something went wrong.

@kgabryje
Copy link
Member Author

kgabryje commented Oct 9, 2020

@zuzana-vej This PR is related to the issue #11096. Could you please verify that it works as you expect?
@graceguo-supercat In the same issue you reported that View results button doesn't work. Could you please verify that it works on this branch?

@codecov-io
Copy link

codecov-io commented Oct 9, 2020

Codecov Report

Merging #11216 into master will decrease coverage by 4.24%.
The diff coverage is 42.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11216      +/-   ##
==========================================
- Coverage   65.62%   61.37%   -4.25%     
==========================================
  Files         834      834              
  Lines       39559    39559              
  Branches     3610     3604       -6     
==========================================
- Hits        25959    24278    -1681     
- Misses      13492    15100    +1608     
- Partials      108      181      +73     
Flag Coverage Δ
#cypress ?
#javascript 62.67% <42.42%> (-0.04%) ⬇️
#python 60.59% <ø> (-0.19%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset-frontend/src/SqlLab/components/App.jsx 61.53% <ø> (-16.24%) ⬇️
...set-frontend/src/SqlLab/components/QuerySearch.jsx 58.33% <33.33%> (-0.89%) ⬇️
...rset-frontend/src/SqlLab/components/QueryTable.jsx 66.66% <44.44%> (+3.87%) ⬆️
superset-frontend/src/SqlLab/App.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/explore/App.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/dashboard/App.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/explore/index.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/dashboard/index.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/setup/setupColors.js 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/chart/ChartContainer.jsx 0.00% <0.00%> (-100.00%) ⬇️
... and 178 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc03549...0cb5b9c. Read the comment docs.

@kgabryje kgabryje changed the title refactor: Refactor QueryTable to use react-table [WIP] refactor: Refactor QueryTable to use react-table [WIP] [depends on #11217] Oct 9, 2020
@kgabryje kgabryje force-pushed the react-table/querytable branch 5 times, most recently from e03cfb0 to aeabbb2 Compare October 12, 2020 15:15

Verified

This commit was signed with the committer’s verified signature.
Julesssss Jules

Verified

This commit was signed with the committer’s verified signature.
Julesssss Jules

Verified

This commit was signed with the committer’s verified signature.
Julesssss Jules

Verified

This commit was signed with the committer’s verified signature.
Julesssss Jules
@kgabryje kgabryje force-pushed the react-table/querytable branch from aeabbb2 to 1064e44 Compare October 15, 2020 12:33
@kgabryje kgabryje changed the title refactor: Refactor QueryTable to use react-table [WIP] [depends on #11217] refactor: Refactor QueryTable to use react-table Oct 15, 2020
@kgabryje kgabryje closed this Oct 15, 2020
@kgabryje kgabryje reopened this Oct 15, 2020
kgabryje and others added 6 commits October 16, 2020 10:09

Verified

This commit was signed with the committer’s verified signature.
Julesssss Jules
Co-authored-by: Evan Rusackas <[email protected]>

Verified

This commit was signed with the committer’s verified signature.
Julesssss Jules
Co-authored-by: Evan Rusackas <[email protected]>

Verified

This commit was signed with the committer’s verified signature.
Julesssss Jules

Verified

This commit was signed with the committer’s verified signature.
Julesssss Jules

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@kgabryje kgabryje closed this Oct 16, 2020
@kgabryje kgabryje reopened this Oct 16, 2020
@mistercrunch mistercrunch merged commit 735123d into apache:master Oct 19, 2020
auxten pushed a commit to auxten/incubator-superset that referenced this pull request Nov 20, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
* Refactor QueryTable to use react-table

* Fix lodash import

* Fix tests

* Fix imports and QuerySearch styles

* Update superset-frontend/src/SqlLab/components/QuerySearch.jsx

Co-authored-by: Evan Rusackas <[email protected]>

* Update superset-frontend/src/SqlLab/components/QuerySearch.jsx

Co-authored-by: Evan Rusackas <[email protected]>

* Lint fix

* Refactored QueryTable into functional component

* Remove calculating content height

* Lint fix

Co-authored-by: Evan Rusackas <[email protected]>
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.0.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/L 🚢 1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants