Replace reactable with DataTable from superset-ui in QueryTable #10981
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@rusackas Would you mind taking a look? |
kgabryje
commented
Sep 21, 2020
|
|
||
| describe('QueryTable', () => { | ||
| // hack for mocking hook that implements sticky behaviour of DataTable | ||
| jest |
There was a problem hiding this comment.
@rusackas This was needed because sticky DataTable uses useLayoutEffect under the hood and that caused enzyme not to render the component properly. If you have a better idea how to solve it I'll be happy to try it!
ktmud
approved these changes
Sep 21, 2020
There was a problem hiding this comment.
Thanks for doing this! I've been wanting to move DataTable to a standalone package, too, so it doesn't feel weird to import things from a plugin in the core UI.
But I think current approach is good. We can always come back and refactor once someone finds time to do the extraction.
nytai
added a commit
to preset-io/superset
that referenced
this pull request
Oct 1, 2020
…le (apache#10981)" This reverts commit e93d92e.
This was referenced Oct 1, 2020
6 tasks
auxten
pushed a commit
to auxten/incubator-superset
that referenced
this pull request
Nov 20, 2020
…he#10981) * Replace reactable with DataTable from superset-ui in QueryTable * Fix tests * Fix pagination * Fix tests
auxten
pushed a commit
to auxten/incubator-superset
that referenced
this pull request
Nov 20, 2020
…ryTable (apache#10981)" (apache#11125) This reverts commit e93d92e.
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
The goal is to replace all uses of
reactable-arclibrary with DataTable from superset-ui (which usesreact-tableunder the hood) and keep the UI/UX mostly unchanged. The reason for that is thatreactable-archas been unsupported for 2 years now, uses deprecated lifecycle methods and is incompatible with Typescript.This PR replaces
reactable-arcin QueryTable componentBEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
After:
As you can see, there's an additional field where we can change the items per page param. Currently
DataTabledoesn't support not showing this field.TEST PLAN
ADDITIONAL INFORMATION