refactor(Select): Migrate Select component to Ant Design 5 #32514

msyavuz · 2025-03-05T16:09:14Z

SUMMARY

Migrate Select component to Ant Design 5. Simplifying the Select component in the process is a secondary objective.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

After:

TESTING INSTRUCTIONS

Run the testing suite and do visual testing.

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

korbit-ai · 2025-03-05T16:09:23Z

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

msyavuz · 2025-03-05T16:10:46Z

/korbit-review

korbit-ai

Review by Korbit AI

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

Category	Issue	Fix Detected
	Typo in property access causing undefined value ▹ view	✅
	Unsafe non-null assertion in mutator call ▹ view	✅
	Ineffective text truncation ▹ view	✅
	Incorrect class name for tag close icon ▹ view

Files scanned

File Path	Reviewed
superset-frontend/src/components/Select/constants.ts	✅
superset-frontend/src/filters/components/TimeGrain/types.ts	✅
superset-frontend/src/filters/components/TimeColumn/types.ts	✅
superset-frontend/packages/superset-ui-chart-controls/src/components/Select.tsx	✅
superset-frontend/plugins/plugin-chart-cartodiagram/src/util/controlPanelUtil.tsx	✅
superset-frontend/src/components/Select/CustomTag.tsx	✅
superset-frontend/src/explore/components/controls/SelectAsyncControl/index.tsx	✅
superset-frontend/src/components/Select/styles.tsx	✅
superset-frontend/src/components/Select/types.ts	✅
superset-frontend/src/components/Select/utils.tsx	✅
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx	✅
superset-frontend/src/components/TableSelector/index.tsx	✅
superset-frontend/src/explore/components/controls/ColorSchemeControl/index.tsx	✅
superset-frontend/src/explore/components/PropertiesModal/index.tsx	✅
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx	✅
superset-frontend/src/components/Select/AsyncSelect.tsx	✅
superset-frontend/src/components/Select/Select.tsx	✅

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

Need a new review? Comment /korbit-review on this PR and I'll review your latest changes.

Korbit Guide: Usage and Customization

Interacting with Korbit

You can manually ask Korbit to review your PR using the /korbit-review command in a comment at the root of your PR.

You can ask Korbit to generate a new PR description using the /korbit-generate-pr-description command in any comment on your PR.

Too many Korbit comments? I can resolve all my comment threads if you use the /korbit-resolve command in any comment on your PR.

On any given comment that Korbit raises on your pull request, you can have a discussion with Korbit by replying to the comment.

Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.

Customizing Korbit

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

Customize Korbit for your organization through the Korbit Console.

Feedback and Support

Tell us what you think of Korbit

Schedule a call with our team

Email us @ support@korbit.ai

superset-frontend/src/components/Select/Select.tsx

superset-frontend/src/explore/components/controls/SelectAsyncControl/index.tsx

korbit-ai · 2025-03-05T16:15:11Z

superset-frontend/src/components/Select/CustomTag.tsx

@@ -69,7 +55,7 @@ export const customTagRender = (props: CustomTagProps) => {
      target.tagName === 'svg' ||
      target.tagName === 'path' ||
      (target.tagName === 'span' &&
-        target.className.includes('ant-tag-close-icon'))
+        target.className.includes('antd5-tag-close-icon'))


Incorrect class name for tag close icon

Tell me more

What is the issue?

The class name check for the close icon is incorrect. Ant Design v5 uses 'ant-tag-close-icon' class, not 'antd5-tag-close-icon'.

Why this matters

This will cause the click propagation prevention to fail when clicking the close icon, leading to unexpected behavior where the dropdown opens when attempting to remove a tag.

Suggested change ∙ Feature Preview

Update the class name check to use the correct Ant Design v5 class:

target.className.includes('ant-tag-close-icon')

_{💬 Chat with Korbit by mentioning @korbit-ai.}

superset-frontend/src/components/Select/styles.tsx

rusackas · 2025-03-05T19:06:11Z

@michael-s-molina and @geido put a lot of work into consolidating numerous Select modules into one Super-Duper-Select. I would check with either/both of them regarding any simplification efforts, just in case there are use cases around the intricacies. Of course if AntD now includes the functionality we built around it previously, or there are unnecessary style layers, then yes, let's go as "vanilla" as we can!

korbit-ai · 2025-04-07T11:13:21Z

I was unable to post the issues I found. This could be because a force push or squash has changed the commit history since I scanned this pull request. You can get another review by commenting /korbit-review.

korbit-ai

Review by Korbit AI

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

Category	Issue	Status
	Arbitrary Virtual Scrolling Threshold ▹ view	✅ Fix detected
	Inefficient Deep Clone of Entire Metadata ▹ view	🧠 Not in scope
	Unreliable Fixed Wait Time ▹ view	🧠 Incorrect
	Complex DOM manipulation needs inline explanation ▹ view	🧠 Not in scope
	Unexplained magic numbers in padding ▹ view	🧠 Not in standard
	Avoid wildcard exports for better API clarity ▹ view	🧠 Not in scope

Files scanned

File Path	Reviewed
superset-frontend/src/components/Select/index.tsx	✅
superset-frontend/src/types/TagType.ts	✅
superset-frontend/src/components/Spin/index.tsx	✅
superset-frontend/src/components/index.ts	✅
superset-frontend/src/components/Select/constants.ts	✅
superset-frontend/src/filters/components/TimeGrain/types.ts	✅
superset-frontend/src/filters/components/TimeColumn/types.ts	✅
superset-frontend/src/components/IndeterminateCheckbox/index.tsx	✅
superset-frontend/src/features/reports/ReportModal/styles.tsx	✅
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DatasetSelect.tsx	✅
superset-frontend/src/components/Select/CustomTag.tsx	✅
superset-frontend/src/explore/components/controls/SelectAsyncControl/index.tsx	✅
superset-frontend/src/GlobalStyles.tsx	✅
superset-frontend/src/components/Icons/AntdEnhanced.tsx	✅
superset-frontend/cypress-base/cypress/e2e/explore/utils.ts	✅
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitleContainer.tsx	✅
superset-frontend/src/components/CronPicker/CronPicker.tsx	✅
superset-frontend/cypress-base/cypress/utils/index.ts	✅
superset-frontend/src/components/Select/styles.tsx	✅
superset-frontend/src/dashboard/actions/nativeFilters.ts	✅
superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/EncryptedField.tsx	✅
superset-frontend/src/explore/components/controls/SelectControl.jsx	✅
superset-frontend/src/components/Select/types.ts	✅
superset-frontend/src/components/Select/Select.stories.tsx	✅
superset-frontend/src/features/tags/TagModal.tsx	✅
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx	✅
superset-frontend/src/components/TableSelector/index.tsx	✅
superset-frontend/src/components/Select/utils.tsx	✅
superset-frontend/src/explore/components/PropertiesModal/index.tsx	✅
superset-frontend/src/components/DatabaseSelector/index.tsx	✅
superset-frontend/src/pages/ChartCreation/index.tsx	✅
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx	✅
superset-frontend/src/features/databases/DatabaseModal/styles.ts	✅
superset-frontend/src/explore/components/controls/ColorSchemeControl/index.tsx	✅
superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx	✅
superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx	✅
superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx	✅
superset-frontend/src/components/Select/AsyncSelect.tsx	✅
superset-frontend/cypress-base/cypress/e2e/dashboard/utils.ts	✅
superset-frontend/src/components/Select/Select.tsx	✅
superset-frontend/cypress-base/cypress/support/directories.ts	✅
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx	✅

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

Need a new review? Comment /korbit-review on this PR and I'll review your latest changes.

Korbit Guide: Usage and Customization

Interacting with Korbit

You can manually ask Korbit to review your PR using the /korbit-review command in a comment at the root of your PR.

You can ask Korbit to generate a new PR description using the /korbit-generate-pr-description command in any comment on your PR.

Too many Korbit comments? I can resolve all my comment threads if you use the /korbit-resolve command in any comment on your PR.

On any given comment that Korbit raises on your pull request, you can have a discussion with Korbit by replying to the comment.

Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.

Customizing Korbit

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

Customize Korbit for your organization through the Korbit Console.

Feedback and Support

Tell us what you think of Korbit

Schedule a call with our team

Email us @ support@korbit.ai

superset-frontend/src/components/Select/index.tsx

+export * from './types';
+export * from './styles';
+export * from './CustomTag';
+export * from './constants';


superset-frontend/src/dashboard/actions/nativeFilters.ts

@@ -122,7 +122,7 @@ export const setInScopeStatusOfFilters =
    // need to update native_filter_configuration in the dashboard metadata
    const metadata = cloneDeep(getState().dashboardInfo.metadata);


superset-frontend/cypress-base/cypress/e2e/explore/utils.ts

superset-frontend/src/GlobalStyles.tsx

+        & .btn-group > .btn {
+          padding: 5px 10px 6px;
+        }


superset-frontend/cypress-base/cypress/utils/index.ts

+  const nativeInputValueSetter = Object.getOwnPropertyDescriptor(
+    window.HTMLInputElement.prototype,
+    'value',
+  )?.set;


superset-frontend/src/components/Select/Select.tsx

          tokenSeparators={tokenSeparators}
+          virtual={fullSelectOptions.length > 20}


msyavuz · 2025-04-08T10:46:10Z

I had to do a ugly workaround for a bug in rc-table which sets removeButton role to "tab". I opened up the issue and the pr for fixing it here:
react-component/tabs#809

geido · 2025-04-08T11:23:14Z

superset-frontend/src/components/Select/Select.tsx

-  isEqual as utilsIsEqual,
+  getSuffixIcon,
+  sortComparatorWithSearchHelper,
+  VIRTUAL_THRESHOLD,


Should this part of constants? And maybe SELECT_ALL_VALUE too?

geido · 2025-04-08T11:24:47Z

superset-frontend/src/components/Select/Select.tsx

@@ -121,9 +124,11 @@ const Select = forwardRef(
      getPopupContainer,
      oneLine,
      maxTagCount: propsMaxTagCount,
+      virtual = undefined,
+      virtualThreshold = VIRTUAL_THRESHOLD,


Any reasons to pass this as a prop? I think if someone wanted to enable the virtual mode they can just do it through the virtual prop.

geido · 2025-04-08T11:25:29Z

superset-frontend/src/components/Select/types.ts

+   * the options will be virtualized.
+   * If the virtual prop is set this will be ignored.
+   */
+  virtualThreshold?: number;


As I wrote above I don't think we need a prop for this

geido · 2025-04-08T11:27:27Z

superset-frontend/src/explore/components/controls/SelectAsyncControl/index.tsx

-            ? mutator(response.json, value)
-            : response.json.result;
-          setOptions(data);
+          if (value) {


@msyavuz not sure if this got missed but it seems we might still ignoring null values with this check which should be valid.

geido

A few nits but looking good overall!

mistercrunch added 11 commits March 3, 2025 17:22

feat: messing with the theme

967cb08

make Cancel secondary

d4d210c

adjusting some buttons

b9c1efa

fix types/tests

c14eaba

fix unit test

9f6b58e

ts-ignore

c12047a

fix a few buttons

6ef5a1c

make secondary filled/primary

ee98803

fix dashboard left panel background-color

7c60a44

fixing storybook

860be7b

set DARK ff to false

b7ea7c5

pull-request-size bot added the size/L label Mar 5, 2025

github-actions bot added plugins packages labels Mar 5, 2025

korbit-ai bot reviewed Mar 5, 2025

View reviewed changes

sadpandajoe added review:checkpoint Last PR reviewed during the daily review standup review:draft labels Mar 5, 2025

sadpandajoe removed the review:checkpoint Last PR reviewed during the daily review standup label Mar 6, 2025

github-actions bot removed plugins packages labels Mar 11, 2025

msyavuz changed the base branch from master to template_less March 13, 2025 17:40

msyavuz added 6 commits March 13, 2025 20:50

refactor(Select): Migrate Select to Ant Design 5

6ed87fa

fix: type and style issues

3485ec3

fix: typo

41d2135

fix: readd some functionality

336f123

fix: readd some more functionality

dd2b8b2

chore: remove unused import & prop

eb443e3

geido requested a review from kgabryje as a code owner April 7, 2025 11:01

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

fix(Tests): Bump Ant Design

3996d69

geido force-pushed the msyavuz/refactor/migrate-select branch from 101a0d2 to 3996d69 Compare April 7, 2025 11:05

msyavuz added 4 commits April 7, 2025 14:50

fix(LeftPanel): test

5401be2

fix(DatabaseSelector): test

0d61494

fix(SqlEditorLeftBar): test

e922ee0

fix(TableSelector): test

353e36c

msyavuz force-pushed the msyavuz/refactor/migrate-select branch from 1713b6e to 353e36c Compare April 7, 2025 17:11

msyavuz added 2 commits April 7, 2025 20:18

Merge branch 'template_less' into msyavuz/refactor/migrate-select

2eb8731

fix: unit tests and virtual over 20

58a40dd

msyavuz marked this pull request as draft April 7, 2025 18:38

msyavuz marked this pull request as ready for review April 7, 2025 18:38

fix: remove unused import

a7eef50

korbit-ai bot reviewed Apr 7, 2025

View reviewed changes

msyavuz added 4 commits April 7, 2025 22:15

fix: refix?

67a314a

refactor: add virtual prop override

ac50e11

Merge branch 'template_less' into msyavuz/refactor/migrate-select

35e0eba

fix: ugly workaround for antd bug

6185475

chore: remove global styles

33f04b4

geido reviewed Apr 8, 2025

View reviewed changes

geido approved these changes Apr 8, 2025

View reviewed changes

fix: remove prop null check

ddb9ab7

msyavuz merged commit b835478 into apache:template_less Apr 8, 2025
41 checks passed

msyavuz deleted the msyavuz/refactor/migrate-select branch April 8, 2025 12:15

michael-s-molina removed the review:draft label May 22, 2025

		@@ -122,7 +122,7 @@ export const setInScopeStatusOfFilters =
		// need to update native_filter_configuration in the dashboard metadata
		const metadata = cloneDeep(getState().dashboardInfo.metadata);

		tokenSeparators={tokenSeparators}
		virtual={fullSelectOptions.length > 20}

refactor(Select): Migrate Select component to Ant Design 5 #32514

refactor(Select): Migrate Select component to Ant Design 5 #32514

Uh oh!

Conversation

msyavuz commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

korbit-ai bot commented Mar 5, 2025

Uh oh!

msyavuz commented Mar 5, 2025

Uh oh!

korbit-ai bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review by Korbit AI

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

Interacting with Korbit

Customizing Korbit

Feedback and Support

Uh oh!

Uh oh!

Uh oh!

korbit-ai bot Mar 5, 2025

Choose a reason for hiding this comment

Incorrect class name for tag close icon

What is the issue?

Why this matters

Suggested change ∙ Feature Preview

Uh oh!

Uh oh!

rusackas commented Mar 5, 2025

Uh oh!

korbit-ai bot commented Apr 7, 2025

Uh oh!

korbit-ai bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review by Korbit AI

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

Interacting with Korbit

Customizing Korbit

Feedback and Support

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

msyavuz commented Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

geido Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

geido Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

geido Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

geido Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

geido left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

msyavuz commented Mar 5, 2025 •

edited

Loading

korbit-ai bot left a comment •

edited

Loading

korbit-ai bot left a comment •

edited

Loading

msyavuz commented Apr 8, 2025 •

edited

Loading