refactor(Table): Use Table instead of html <table> in CollectionTable #33159

msyavuz · 2025-04-17T07:59:01Z

SUMMARY

Refactor DatasourceEditor and CollectionTable components to use our Table component instead of html table.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

After:

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

…asource

korbit-ai · 2025-04-17T07:59:08Z

Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment /korbit-review.

Your admin can change your review schedule in the Korbit Console

korbit-ai

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Category	Issue	Status
	Inefficient Expanded Rows Calculation ▹ view	🧠 Not in standard

Files scanned

File Path	Reviewed
superset-frontend/src/components/Datasource/types.ts	✅
superset-frontend/src/components/Table/index.tsx	✅
superset-frontend/cypress-base/cypress/support/directories.ts	✅
superset-frontend/src/components/Datasource/CollectionTable.tsx	✅
superset-frontend/src/components/Datasource/DatasourceEditor.jsx	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

superset-frontend/src/components/Datasource/CollectionTable.tsx

+    const expandedRowKeys = Object.keys(this.state.expandedColumns).filter(
+      id => this.state.expandedColumns[id],
    );


github-actions · 2025-04-22T14:37:47Z

@EnxDev Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments

github-actions · 2025-04-22T14:47:18Z

@EnxDev Ephemeral environment spinning up at http://35.89.175.154:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.

EnxDev · 2025-04-22T14:59:43Z

superset-frontend/src/components/Datasource/CollectionTable.tsx

@@ -113,6 +56,32 @@ const StyledButtonWrapper = styled.span`
  `}
 `;

+type CollectionItem = { id: string | number; [key: string]: any };


Would it be possible to define a more specific type here if we had better knowledge of the properties, to avoid any type?

I noticed there are quite a few other any types in the code — can we try to avoid them as well?

I need generics for this type but the only component that uses this is jsx so generics don't do anything. But we should definitely consider replacing these with proper types when DatasourceEditor is typescript

Yeah I think that's fine. The original intent when I built this a long time ago was for it to become the basis for all CRUD in the app, but ended up going a different way. If anything in the future, we'd remove this base component and go more towards what other CRUDs are using, if that makes sense. For now we'll flag as "needs refactor" and can do minimal edits to get theming through.

EnxDev · 2025-04-22T15:08:29Z

superset-frontend/src/components/Datasource/CollectionTable.tsx

+            )}
+          </>
+        ),
+        render: (text: any, record: CollectionItem) =>


here as well

Same thing here, this could be anything and best way to handle this is through generics which we can't do.

geido · 2025-04-23T13:13:16Z

superset-frontend/src/components/Datasource/CollectionTable.tsx

+  ColumnsType,
+  SortOrder,
+  SorterResult,
+  TablePaginationConfig,


These need to be imported as type for proper tree shaking

geido · 2025-04-23T13:21:17Z

From manual testing

The Table could benefit for more space as it is available

Not sure if related but spacing is wrong

There is a space when scrolling showing what's behind

When typing it suddenly re-orders

Superset.mp4

This is weird. When first loading the Superset instance (no cache - try with Incognito), this is what happens for a few seconds before fixing itself

There might be more but I'll give you a chance to look into these first

github-actions · 2025-04-25T06:55:22Z

@EnxDev Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments

github-actions · 2025-04-25T07:07:08Z

@EnxDev Ephemeral environment spinning up at http://44.247.53.125:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.

…asource

mistercrunch

LGTM

EnxDev · 2025-04-25T17:57:04Z

I believe this bug was introduced, could we fix it in a separate PR?

Recording.2025-04-25.195442.mp4

msyavuz added 3 commits April 16, 2025 11:10

refactor: use our table component

db5721b

Merge branch 'template_less' into msyavuz/refactor/use-ant-tables-dat…

045ea7f

…asource

fix: types

5123524

msyavuz changed the base branch from master to template_less April 17, 2025 07:59

pull-request-size bot added the size/XL label Apr 17, 2025

justinpark added the review:draft label Apr 17, 2025

justinpark self-requested a review April 17, 2025 17:18

msyavuz added 6 commits April 19, 2025 10:40

fix: type issue

3518d71

fix: lint and test

77e06d8

chore: layout changes and test fixes

1883ca6

fix: change expand row name

84f068e

fix: e2e tests and new item order

ae6b661

fix: match id types, set disabled correctly

10898d8

msyavuz marked this pull request as ready for review April 22, 2025 13:07

msyavuz requested review from sadpandajoe, geido, eschutho, rusackas, betodealmeida and mistercrunch as code owners April 22, 2025 13:07

msyavuz removed the review:draft label Apr 22, 2025

dosubot bot added change:frontend Requires changing the frontend frontend:refactor Related to refactoring the frontend labels Apr 22, 2025

korbit-ai bot reviewed Apr 22, 2025

View reviewed changes

superset-frontend/src/components/Datasource/CollectionTable.tsx

Comment on lines +371 to 373

const expandedRowKeys = Object.keys(this.state.expandedColumns).filter(

id => this.state.expandedColumns[id],

);

This comment was marked as resolved.

Sign in to view

EnxDev added the testenv-up label Apr 22, 2025

EnxDev reviewed Apr 22, 2025

View reviewed changes

geido reviewed Apr 23, 2025

View reviewed changes

msyavuz added 3 commits April 24, 2025 12:46

fix: column widths

4653a3c

fix: import as types

601c9f1

fix: sticky header overflow

e957b22

EnxDev added testenv-up and removed testenv-up labels Apr 25, 2025

msyavuz added 2 commits April 25, 2025 18:40

fix(CollectionTable): preserve order while editing

1578372

Merge branch 'template_less' into msyavuz/refactor/use-ant-tables-dat…

d551f42

…asource

mistercrunch approved these changes Apr 25, 2025

View reviewed changes

mistercrunch merged commit a7b7e63 into template_less Apr 25, 2025
1 check passed

mistercrunch deleted the msyavuz/refactor/use-ant-tables-datasource branch April 25, 2025 16:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(Table): Use Table instead of html <table> in CollectionTable #33159

refactor(Table): Use Table instead of html <table> in CollectionTable #33159

msyavuz commented Apr 17, 2025 •

edited

Loading

korbit-ai bot commented Apr 17, 2025

korbit-ai bot left a comment •

edited

Loading

This comment was marked as resolved.

github-actions bot commented Apr 22, 2025

github-actions bot commented Apr 22, 2025

EnxDev Apr 22, 2025 •

edited

Loading

msyavuz Apr 24, 2025

mistercrunch Apr 25, 2025

EnxDev Apr 22, 2025

msyavuz Apr 22, 2025

geido Apr 23, 2025

geido commented Apr 23, 2025

github-actions bot commented Apr 25, 2025

github-actions bot commented Apr 25, 2025

mistercrunch left a comment

EnxDev commented Apr 25, 2025

refactor(Table): Use Table instead of html <table> in CollectionTable #33159

refactor(Table): Use Table instead of html <table> in CollectionTable #33159

Conversation

msyavuz commented Apr 17, 2025 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

korbit-ai bot commented Apr 17, 2025

korbit-ai bot left a comment • edited Loading

Choose a reason for hiding this comment

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

This comment was marked as resolved.

github-actions bot commented Apr 22, 2025

github-actions bot commented Apr 22, 2025

EnxDev Apr 22, 2025 • edited Loading

Choose a reason for hiding this comment

msyavuz Apr 24, 2025

Choose a reason for hiding this comment

mistercrunch Apr 25, 2025

Choose a reason for hiding this comment

EnxDev Apr 22, 2025

Choose a reason for hiding this comment

msyavuz Apr 22, 2025

Choose a reason for hiding this comment

geido Apr 23, 2025

Choose a reason for hiding this comment

geido commented Apr 23, 2025

github-actions bot commented Apr 25, 2025

github-actions bot commented Apr 25, 2025

mistercrunch left a comment

Choose a reason for hiding this comment

EnxDev commented Apr 25, 2025

msyavuz commented Apr 17, 2025 •

edited

Loading

korbit-ai bot left a comment •

edited

Loading

EnxDev Apr 22, 2025 •

edited

Loading