refactor(Collapse): Upgrade Collapse to Antd5 #32959

DamianPendrak · 2025-04-01T12:28:38Z

SUMMARY

Fixes #32542

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Home

Before

After

SQL Editor Table Element

Before

After

Explore Data

Before

After

Connect a database - Advanced tab

Before

After

Upload modal

Before

After

Add Alert / Report

Before

After

Fixed or metric

Before

After

Dashboard filters

Before

After

Dashboard Cross-filters

Before

After

Dasbhoard - Out of scope filters

Before

After

VizTypeGallery

Before

After

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue: Ant Design 5: Upgrade Collapse component #32542
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-04-01T12:28:43Z

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

geido · 2025-04-02T11:16:37Z

superset-frontend/src/SqlLab/components/TableElement/index.tsx

-                aria-label="Copy"
-                tooltip={t('Copy SELECT statement to the clipboard')}
+                className={
+                  `fa fa-sort-${sortColumns ? 'numeric' : 'alpha'}-asc ` +


We are removing usages of fa icons from the codebase. Can you use the Icons component from src/components/Icons instead?

geido · 2025-04-02T11:17:44Z

superset-frontend/src/SqlLab/components/TableElement/index.tsx

+                <CopyToClipboard
+                  copyNode={
+                    <IconTooltip
+                      aria-label="Copy"


Let's localize this too. I see inconsistent localization with this prop across the codebase but I guess localizing won't hurt.

geido · 2025-04-02T11:19:10Z

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

-          expandIconPosition === 'right' &&
-          `
-            .anticon.anticon-right.ant-collapse-arrow > svg {
+      ${({ expandIconPosition }) =>


This is all necessary because Ant Design does not support arrows on the right?

It does, but the arrow points to the right by default. In Superset, when the arrow is on the right, it points down when closed and up when open

geido · 2025-04-02T11:20:30Z

superset-frontend/src/explore/components/controls/FixedOrMetricControl/index.jsx

-        </Collapse>
+          items={[
+            {
+              key: '1',


Can we use human-readable keys?

geido · 2025-04-02T11:22:09Z

...t-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.tsx

+    <Collapse
+      items={[
+        {
+          key: '1',


human-readable if possible

geido · 2025-04-02T11:22:34Z

superset-frontend/src/pages/Home/index.tsx

-                  {!queryData ? (
+              items={[
+                {
+                  key: '1',


same comment about human-readable

msyavuz · 2025-04-02T20:04:08Z

superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx

+          {tables.map(table => (
+            <TableElement
+              table={table}
+              key={table.id}
+              activeKey={tables
+                .filter(({ expanded }) => expanded)
+                .map(({ id }) => id)}
+              onChange={onToggleTable}
+            />
+          ))}


I am not sure why this refactor of Collapse into TableElement necessary?

Mainly because of the hover functionality for the tooltip and the fact that the Collapse items prop accepts label and children as separate components

msyavuz · 2025-04-02T20:06:57Z

superset-frontend/src/SqlLab/components/TableElement/index.tsx

+              >
+                <Icons.SyncOutlined
+                  iconSize="m"
+                  iconColor={theme.colors.grayscale.dark5}


I think we are trying to use Ant Design tokens whenever we can so i am not sure but this would roughly map to theme.colorIcon?

Okay, I thought that the contrast is too low with the theme.colorIcon. I will update the code, but let me know if something changes because of the contrast

msyavuz · 2025-04-02T20:07:23Z

superset-frontend/src/SqlLab/components/TableElement/index.tsx

                  aria-hidden
+                  iconColor={
+                    sortColumns
+                      ? theme.colors.grayscale.dark5


msyavuz · 2025-04-02T20:07:38Z

superset-frontend/src/SqlLab/components/TableElement/index.tsx

+                      <Icons.CopyOutlined
+                        iconSize="m"
+                        aria-hidden
+                        iconColor={theme.colors.grayscale.dark5}


msyavuz · 2025-04-02T20:07:45Z

superset-frontend/src/SqlLab/components/TableElement/index.tsx

+                <Icons.CloseOutlined
+                  iconSize="m"
+                  aria-hidden
+                  iconColor={theme.colors.grayscale.dark5}


msyavuz · 2025-04-02T20:09:39Z

superset-frontend/src/components/ErrorMessage/MarshmallowErrorMessage.tsx

@@ -41,9 +41,11 @@ const collapseStyle = (theme: SupersetTheme) => css`
  .ant-collapse-arrow {
    left: 0px !important;
  }
+
  .ant-collapse-header {


Won't these classnames change to antd5?

msyavuz · 2025-04-02T20:12:55Z

...ashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx

+                  {
+                    key: `${filterId}-${FilterPanels.configuration.key}`,
+                    label: FilterPanels.configuration.name,
+                    children: (


Is losing forceRender not important here?

Restored the forceRender

geido · 2025-04-03T12:46:31Z

superset-frontend/src/SqlLab/components/ColumnElement/index.tsx

@@ -105,7 +105,7 @@ const ColumnElement = ({ column }: ColumnElementProps) => {
  }
  return (
    <div className="clearfix table-column">
-      <div className="pull-left m-l-10 col-name">
+      <div className="pull-left col-name">


Do we know where these classes are coming from? Can we use the standard Ant Design approach here, like Flex or Space?

geido · 2025-04-03T12:49:57Z

superset-frontend/src/SqlLab/components/TableElement/index.tsx

-      }
-    }
-  `}
+const StyledControls = styled.div`


Don't want to go too much outside of the scope of the PR but do you think we can use standard Ant Design Flex and Space components to manage the layout for all the styles in this file?

geido · 2025-04-03T12:51:52Z

...-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/VerticalCollapse.tsx

+                  width: 100%;
+                  height: 1px;
+                  display: block;
+                  background: ${theme.colors.grayscale.light3};


Do we need this? Can we use an Ant Design color instead? Also, this should be a standard atomic component?

mistercrunch · 2025-04-03T18:21:55Z

@DamianPendrak I see some pre-commit checks fail, highly recommend setting up pre-commit -> https://superset.apache.org/docs/contributing/development/#git-hooks

Once you do, you may want to squish your commits as a way to re-trigger it on all the files you touched in this PR.

mistercrunch · 2025-04-03T18:24:04Z

Also wondering what's required to do in order for going from a DRAFT PR to "ready for review"

DamianPendrak · 2025-04-03T20:33:50Z

Also wondering what's required to do in order for going from a DRAFT PR to "ready for review"

I think I will fix the failing tests and then mark it ready

korbit-ai · 2025-04-04T08:50:43Z

Korbit doesn't automatically review large (3000+ lines changed) pull requests such as this one. If you want me to review anyway, use /korbit-review.

geido · 2025-04-07T14:16:00Z

The color of the subtitle seems hard to see. Is this a vanilla Ant Design implementation?
When I first visited the homepage I found all collapse closed. I don't believe I closed them. Something to check.
Colors of text inside the collapse for filters is off. Is it because of Checkbox? Currently being developed here

You have a conflict. Thanks for the hard work on this!

geido

Approving as it looks mostly good with comments I have left above. It would be great to have more eyes on testing as well. @msyavuz maybe you can help with that.

Thank you!

pull-request-size bot added the size/XXL label Apr 1, 2025

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

sadpandajoe linked an issue Apr 1, 2025 that may be closed by this pull request

Ant Design 5: Upgrade Collapse component #32542

Open

geido added preset:bounty Issues that have been selected by Preset and have a bounty attached. hold! On hold labels Apr 2, 2025

geido reviewed Apr 2, 2025

View reviewed changes

msyavuz self-requested a review April 2, 2025 11:29

DamianPendrak force-pushed the collapse-component-upgrade branch from 2eb2f43 to a331275 Compare April 2, 2025 13:00

sadpandajoe removed the review:checkpoint Last PR reviewed during the daily review standup label Apr 2, 2025

trentlavoie mentioned this pull request Apr 2, 2025

fix(frontend): add missing antd-5 icon to import #32990

Merged

9 tasks

msyavuz reviewed Apr 2, 2025

View reviewed changes

geido reviewed Apr 3, 2025

View reviewed changes

DamianPendrak marked this pull request as ready for review April 4, 2025 08:50

DamianPendrak requested review from sadpandajoe, eschutho, rusackas, betodealmeida and mistercrunch as code owners April 4, 2025 08:50

dosubot bot added change:frontend Requires changing the frontend frontend:refactor:antd5 labels Apr 4, 2025

Collapse component upgrade to antd5

bb7a638

DamianPendrak force-pushed the collapse-component-upgrade branch from 0d03d4d to bb7a638 Compare April 4, 2025 10:09

geido approved these changes Apr 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(Collapse): Upgrade Collapse to Antd5 #32959

refactor(Collapse): Upgrade Collapse to Antd5 #32959

DamianPendrak commented Apr 1, 2025 •

edited

Loading

korbit-ai bot commented Apr 1, 2025

geido Apr 2, 2025

geido Apr 2, 2025

geido Apr 2, 2025

DamianPendrak Apr 2, 2025

geido Apr 2, 2025

geido Apr 2, 2025

geido Apr 2, 2025

msyavuz Apr 2, 2025

DamianPendrak Apr 3, 2025

msyavuz Apr 2, 2025

DamianPendrak Apr 3, 2025

msyavuz Apr 2, 2025

msyavuz Apr 2, 2025

msyavuz Apr 2, 2025

msyavuz Apr 2, 2025

msyavuz Apr 2, 2025

DamianPendrak Apr 3, 2025

geido Apr 3, 2025

geido Apr 3, 2025

geido Apr 3, 2025

mistercrunch commented Apr 3, 2025 •

edited

Loading

mistercrunch commented Apr 3, 2025

DamianPendrak commented Apr 3, 2025

korbit-ai bot commented Apr 4, 2025

geido commented Apr 7, 2025

geido left a comment

refactor(Collapse): Upgrade Collapse to Antd5 #32959

Are you sure you want to change the base?

refactor(Collapse): Upgrade Collapse to Antd5 #32959

Conversation

DamianPendrak commented Apr 1, 2025 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Home

Before

After

SQL Editor Table Element

Before

After

Explore Data

Before

After

Connect a database - Advanced tab

Before

After

Upload modal

Before

After

Add Alert / Report

Before

After

Fixed or metric

Before

After

Dashboard filters

Before

After

Dashboard Cross-filters

Before

After

Dasbhoard - Out of scope filters

Before

After

VizTypeGallery

Before

After

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

korbit-ai bot commented Apr 1, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mistercrunch commented Apr 3, 2025 • edited Loading

mistercrunch commented Apr 3, 2025

DamianPendrak commented Apr 3, 2025

korbit-ai bot commented Apr 4, 2025

geido commented Apr 7, 2025

geido left a comment

Choose a reason for hiding this comment

DamianPendrak commented Apr 1, 2025 •

edited

Loading

mistercrunch commented Apr 3, 2025 •

edited

Loading