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

feat: create table component based on ant design Table #21520

Conversation

eric-briscoe
Copy link
Contributor

@eric-briscoe eric-briscoe commented Sep 19, 2022

Adds a new Table component based on Ant Design as part of movement to a Superset Design system

SUMMARY

To view the table component and see the documentation:
https://75b68f5--62b4d4c211d03654c1992245.chromatic.com/?path=/story/design-system-components-table-overview--page (manually updating to Chromatic)

To run local:

  1. checkout this branch
  2. open a terminal at the superset/superset-frontend directory and run npm ci
  3. in the same terminal run npm run storybook
  4. Open http://localhost:6006/?path=/docs/design-system-components-table-overview--page in your browser

This provides the documentation to use the new Table component as well as interactive stories to see the behavior of the various props

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@eric-briscoe eric-briscoe changed the title Ericbriscoe/sc 51384/create table component based on antd feat: create table component based on antd Table Oct 21, 2022
@eric-briscoe eric-briscoe marked this pull request as ready for review October 21, 2022 22:56
@eric-briscoe eric-briscoe changed the title feat: create table component based on antd Table feat: create table component based on ant design Table Oct 24, 2022
@codecov
Copy link

codecov bot commented Oct 24, 2022

Codecov Report

Merging #21520 (1a25f33) into master (cd1b379) will increase coverage by 0.01%.
The diff coverage is 56.77%.

@@            Coverage Diff             @@
##           master   #21520      +/-   ##
==========================================
+ Coverage   67.07%   67.09%   +0.01%     
==========================================
  Files        1815     1827      +12     
  Lines       69575    69876     +301     
  Branches     7486     7548      +62     
==========================================
+ Hits        46665    46880     +215     
- Misses      20974    21040      +66     
- Partials     1936     1956      +20     
Flag Coverage Δ
javascript 53.72% <56.77%> (+0.14%) ⬆️

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

Impacted Files Coverage Δ
superset-frontend/src/components/Button/index.tsx 100.00% <ø> (ø)
...rc/components/Table/utils/InteractiveTableUtils.ts 36.44% <36.44%> (ø)
...ponents/Table/cell-renderers/NumericCell/index.tsx 50.00% <50.00%> (ø)
superset-frontend/src/components/Table/index.tsx 65.07% <65.07%> (ø)
...uperset-frontend/src/components/Dropdown/index.tsx 95.83% <83.33%> (-4.17%) ⬇️
...mponents/Table/cell-renderers/ActionCell/index.tsx 95.83% <95.83%> (ø)
superset-frontend/src/components/Loading/index.tsx 100.00% <100.00%> (ø)
...onents/Table/cell-renderers/ActionCell/fixtures.ts 100.00% <100.00%> (ø)
...mponents/Table/cell-renderers/ButtonCell/index.tsx 100.00% <100.00%> (ø)
...nd/src/components/Table/cell-renderers/fixtures.ts 100.00% <100.00%> (ø)
... and 13 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@AAfghahi
Copy link
Member

AAfghahi commented Nov 3, 2022

Approved but have a few questions!

Copy link
Member

@lyndsiWilliams lyndsiWilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went through and found some things/had some questions. I also noticed that a storybook log was pushed up to superset-frontend/build-storybook.log. I don't see storybook logs elsewhere in the codebase, was this log meant to be part of this PR?

superset-frontend/src/components/Table/Table.overview.mdx Outdated Show resolved Hide resolved
superset-frontend/src/components/Table/Table.overview.mdx Outdated Show resolved Hide resolved
superset-frontend/src/components/Table/Table.overview.mdx Outdated Show resolved Hide resolved
superset-frontend/src/components/Table/Table.overview.mdx Outdated Show resolved Hide resolved
superset-frontend/src/components/Table/Table.overview.mdx Outdated Show resolved Hide resolved
superset-frontend/src/components/Table/index.tsx Outdated Show resolved Hide resolved
superset-frontend/src/components/Table/sorters.test.ts Outdated Show resolved Hide resolved
superset-frontend/src/components/Table/utils/utils.ts Outdated Show resolved Hide resolved
eric-briscoe and others added 3 commits November 7, 2022 10:41
Co-authored-by: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com>
Co-authored-by: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com>
Co-authored-by: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com>
eric-briscoe and others added 15 commits November 7, 2022 16:32
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
…ell/index.tsx

Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
…ell/index.tsx

Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
…ll/ActionCell.overview.mdx

Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
…ll/ActionCell.overview.mdx

Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
…ll/ActionCell.overview.mdx

Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
@eric-briscoe
Copy link
Contributor Author

eric-briscoe commented Nov 8, 2022

I went through and found some things/had some questions. I also noticed that a storybook log was pushed up to superset-frontend/build-storybook.log. I don't see storybook logs elsewhere in the codebase, was this log meant to be part of this PR?

@lyndsiWilliams I removed that file, thanks for catching that!

eric-briscoe and others added 3 commits November 8, 2022 13:50
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
Copy link
Member

@michael-s-molina michael-s-molina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for all the hard work and for addressing the comments @eric-briscoe!

@eric-briscoe
Copy link
Contributor Author

LGTM. Thanks for all the hard work and for addressing the comments @eric-briscoe!

Thank you for the thorough reviews @michael-s-molina @lyndsiWilliams @AAfghahi @pkdotson 😄

@AAfghahi AAfghahi merged commit 736b534 into apache:master Nov 9, 2022
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 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/XXL 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants