refactor(Popover): Upgrade Popover to Antd5 #31973

alexandrusoare · 2025-01-23T13:58:23Z

SUMMARY

Upgrading Popover component to Antd5.

Fixes #31953

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE

AFTER

BEFORE

AFTER

BEFORE

AFTER

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

korbit-ai · 2025-01-23T13:58:27Z

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	Fix Detected
	Filter popover z-index issue in full-size mode ▹ view	✅
	Redundant Z-Index Configuration ▹ view	✅
	Unreliable Header Access ▹ view

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.

Chat with Korbit on issues we post by tagging @korbit-ai in your reply.

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 @ [email protected]

superset-frontend/cypress-base/cypress/support/e2e.ts

superset-frontend/src/theme/index.ts

superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx

superset-frontend/cypress-base/cypress/e2e/dashboard/horizontalFilterBar.test.ts

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

geido · 2025-01-27T10:34:50Z

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

-// for now we're just redirecting
-export { Popover as default } from './Popover';
+export interface PopoverProps extends AntdPopoverProps {
+  forceRender?: boolean;


What is this for? I don't see forceRender exposed in the Popover props of Ant Design. Will this have any effect?

This was being used before, I just combined the Popover.tsx file with the index.tsx file -> https://github.com/apache/superset/pull/31973/files/18ef69e7e88cc0389c4dd991050e176988acb9e8#diff-575bb0af856e1f113959bcf0ec737a412cb23b3b5ed154012d993d5be95da187
And the forceRender prop is being used in some places, here's 1 for example: superset/superset-frontend/src/components/DropdownContainer/index.tsx

I am not sure this answers my question. If forceRender is not an option that Popover accepts in Ant Design, what is the use of it? Is this actually doing anything? Can you confirm that, please?

The forceRender option ensures that the Popover’s content is rendered in the DOM even when the Popover is not opened. I verified that it behaves as expected, which confirms that it works even though it isn’t an official prop in Ant Design.

superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx

superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/index.tsx

superset-frontend/src/explore/components/DataTableControl/index.tsx

EnxDev · 2025-01-27T22:44:12Z

@alexandrusoare regarding Popover.stories.tsx, could you add the default values for TRIGGERS i think 'click, 'top' for PLACEMENTS, and include a state that changes based on the selected triggers?

For example, when you click the button, the popover should open, or it should change when the selected trigger is changed, for make it interactive.

Recording.2025-01-27.234116.mp4

superset-frontend/src/theme/index.ts

msyavuz

Looks good, minor gripe for me is default exports but i've seen it used in other places so i think it's fine

kgabryje · 2025-01-30T16:41:49Z

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

@@ -181,11 +181,13 @@ const DropdownContainer = forwardRef(
    );

    useLayoutEffect(() => {
+      if (popoverVisible) {


should we add it to the dependency array?

No, I don't think so, I added this statement because of a visual bug that was happening in certain cases. And I think that we don't need to recalculate the size when popover is closed .

superset-frontend/src/dashboard/components/nativeFilters/FilterCard/index.tsx

superset-frontend/src/explore/components/controls/MapViewControl/MapViewPopoverContent.tsx

geido · 2025-02-03T16:52:04Z

On top of the comments I have unresolved above, can you please check if this is a newly introduced bug? It seems the Popover would not open in the right position, making it hard to interact with when within "More filters" in horizontal mode.

geido · 2025-02-03T16:59:02Z

superset-frontend/src/dashboard/components/nativeFilters/FilterCard/index.tsx

+      overlayStyle={{
+        width: '240px',
+        padding: 0,
+        borderRadius: theme.borderRadius,


Can we rely on the default borders of Ant Design here? Same with paddings?

Yes, I think we can rely on the defaults here

Without Padding:

With Padding:

Without Border:

With Border:

alexandrusoare added 7 commits January 21, 2025 10:14

refactor(Popover): upgrade Popover to antd5

be1d564

refactor(DetailsPanel): removing custom styles

f88fa0e

refactor(components): upgrading to andt5

e6cc1fe

refactor(popover): updating to Antd5

636c1a1

refactor(popover): updating to Antd5

3c99830

refactor(Popover): upgrading to antd5

a69a8b0

refactor(Popover): upgrading to antd5

584239b

pull-request-size bot added the size/L label Jan 23, 2025

github-actions bot added the packages label Jan 23, 2025

rusackas added the review:draft label Jan 23, 2025

alexandrusoare and others added 7 commits January 24, 2025 10:23

Merge branch 'master' into alexandrusoare/refactor/antd5-popover

2839455

fix(e2e): fixed e2e test

74d9a30

fix(e2e): fixed e2e test

40a2034

feat(story): added props to storybook

cabf53f

feat(styles): removed some styles

159a7fb

feat(style): removed some styles

4677efa

feat(style): removed some styles

18ef69e

alexandrusoare marked this pull request as ready for review January 24, 2025 13:04

alexandrusoare requested review from sadpandajoe, geido, eschutho, rusackas, betodealmeida, michael-s-molina and kgabryje as code owners January 24, 2025 13:04

dosubot bot added change:frontend Requires changing the frontend frontend:refactor:antd5 labels Jan 24, 2025

korbit-ai bot reviewed Jan 24, 2025

View reviewed changes

superset-frontend/cypress-base/cypress/support/e2e.ts Show resolved Hide resolved

superset-frontend/src/theme/index.ts Outdated Show resolved Hide resolved

superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx Outdated Show resolved Hide resolved

michael-s-molina removed the review:draft label Jan 24, 2025

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

geido reviewed Jan 27, 2025

View reviewed changes

EnxDev self-requested a review January 27, 2025 22:33

geido reviewed Jan 28, 2025

View reviewed changes

superset-frontend/src/theme/index.ts Outdated Show resolved Hide resolved

alexandrusoare added 3 commits January 28, 2025 12:55

refactor(Popover): small changes from PR reviews

ad18747

fix(bug): fixed bug related to dropdowncontainer

e70f6a4

chore(clean): small cleaning up

491a643

msyavuz approved these changes Jan 30, 2025

View reviewed changes

kgabryje reviewed Jan 30, 2025

View reviewed changes

superset-frontend/src/dashboard/components/nativeFilters/FilterCard/index.tsx Outdated Show resolved Hide resolved

kgabryje reviewed Jan 30, 2025

View reviewed changes

superset-frontend/src/explore/components/controls/MapViewControl/MapViewPopoverContent.tsx Show resolved Hide resolved

refactor(pr): small refactor from pr reviews

67a7571

geido reviewed Feb 3, 2025

View reviewed changes

refactor(dropdowncontainer): small changes for PR reviews

79988e4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(Popover): Upgrade Popover to Antd5 #31973

refactor(Popover): Upgrade Popover to Antd5 #31973

alexandrusoare commented Jan 23, 2025 •

edited by michael-s-molina

Loading

korbit-ai bot commented Jan 23, 2025

korbit-ai bot left a comment •

edited

Loading

geido Jan 27, 2025

alexandrusoare Jan 28, 2025

geido Feb 3, 2025 •

edited

Loading

alexandrusoare Feb 4, 2025

EnxDev commented Jan 27, 2025 •

edited

Loading

msyavuz left a comment

kgabryje Jan 30, 2025

alexandrusoare Jan 31, 2025

geido commented Feb 3, 2025

geido Feb 3, 2025

alexandrusoare Feb 4, 2025

refactor(Popover): Upgrade Popover to Antd5 #31973

Are you sure you want to change the base?

refactor(Popover): Upgrade Popover to Antd5 #31973

Conversation

alexandrusoare commented Jan 23, 2025 • edited by michael-s-molina Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE

AFTER

BEFORE

AFTER

BEFORE

AFTER

BEFORE

AFTER

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

korbit-ai bot commented Jan 23, 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.

Interacting with Korbit

Customizing Korbit

Feedback and Support

geido Jan 27, 2025

Choose a reason for hiding this comment

alexandrusoare Jan 28, 2025

Choose a reason for hiding this comment

geido Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

alexandrusoare Feb 4, 2025

Choose a reason for hiding this comment

EnxDev commented Jan 27, 2025 • edited Loading

msyavuz left a comment

Choose a reason for hiding this comment

kgabryje Jan 30, 2025

Choose a reason for hiding this comment

alexandrusoare Jan 31, 2025

Choose a reason for hiding this comment

geido commented Feb 3, 2025

geido Feb 3, 2025

Choose a reason for hiding this comment

alexandrusoare Feb 4, 2025

Choose a reason for hiding this comment

alexandrusoare commented Jan 23, 2025 •

edited by michael-s-molina

Loading

korbit-ai bot left a comment •

edited

Loading

geido Feb 3, 2025 •

edited

Loading

EnxDev commented Jan 27, 2025 •

edited

Loading