refactor(Alert): Migrate Alert component to Ant Design V5 #31168

LevisNgigi · 2024-11-26T17:23:13Z

SUMMARY

This PR upgrades the Alert component to Ant Design V5

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before

After

TESTING INSTRUCTIONS

Trigger an alert to Verify the appearance of alerts throughout the application.

ADDITIONAL INFORMATION

Has associated issue: Fixes Ant Design 5: Upgrade Alert component #30911
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

…t into upgrade-alert-antd-v5

geido

Thanks for the contribution. I gave it a first-pass review. I am wondering if we need to upgrade more references to the updated class selectors throughout the code ant5-{something} for the Alert component.

geido · 2024-11-27T10:45:16Z

superset-frontend/cypress-base/cypress/e2e/explore/visualizations/line.test.ts

@@ -62,7 +62,7 @@ describe('Visualization > Line', () => {
      'not.exist',
    );

-    cy.get('.ant-alert-warning').should('not.exist');
+    cy.get('.antd-v5-alert-warning').should('not.exist');


Suggested change

cy.get('.antd-v5-alert-warning').should('not.exist');

cy.get('.antd5-alert-warning').should('not.exist');

geido · 2024-11-27T10:45:26Z

superset-frontend/cypress-base/cypress/e2e/explore/visualizations/line.test.ts

@@ -71,7 +71,7 @@ describe('Visualization > Line', () => {
    cy.get('#controlSections-tab-display').click();
    cy.get('span').contains('Y Axis Bounds').scrollIntoView();
    cy.get('input[placeholder="Min"]').type('-0.1', { delay: 100 });
-    cy.get('.ant-alert-warning').should('not.exist');
+    cy.get('.antd-v5-alert-warning').should('not.exist');


Suggested change

cy.get('.antd-v5-alert-warning').should('not.exist');

cy.get('.antd5-alert-warning').should('not.exist');

geido · 2024-11-27T10:45:38Z

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

@@ -94,7 +94,7 @@ export const databasesPage = {
    dbDropdown: '[class="ant-select-selection-search-input"]',
    dbDropdownMenu: '.rc-virtual-list-holder-inner',
    dbDropdownMenuItem: '[class="ant-select-item-option-content"]',
-    infoAlert: '.ant-alert',
+    infoAlert: '.antd-v5-alert',


Suggested change

infoAlert: '.antd-v5-alert',

infoAlert: '.antd5-alert',

geido · 2024-11-27T10:45:45Z

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

@@ -103,7 +103,7 @@ export const databasesPage = {
      helperBottom: '.helper-bottom',
      postgresDatabase: '[name="database"]',
      dbInput: '[name="database_name"]',
-      alertMessage: '.ant-alert-message',
+      alertMessage: '.antd-v5-alert-message',


Suggested change

alertMessage: '.antd-v5-alert-message',

alertMessage: '.antd5-alert-message',

geido · 2024-11-27T10:45:56Z

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

@@ -166,7 +166,7 @@ export const sqlLabView = {
  renderedTableHeader: '.ReactVirtualized__Table__headerRow',
  renderedTableRow: '.ReactVirtualized__Table__row',
  errorBody: '.error-body',
-  alertMessage: '.ant-alert-message',
+  alertMessage: '.antd-v5-alert-message',


Suggested change

alertMessage: '.antd-v5-alert-message',

alertMessage: '.antd5-alert-message',

geido · 2024-11-27T10:46:04Z

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

@@ -325,7 +325,7 @@ export const nativeFilters = {
    confirmCancelButton: dataTestLocator(
      'native-filter-modal-confirm-cancel-button',
    ),
-    alertXUnsavedFilters: '.ant-alert-message',
+    alertXUnsavedFilters: '.antd-v5-alert-message',


Suggested change

alertXUnsavedFilters: '.antd-v5-alert-message',

alertXUnsavedFilters: '.antd5-alert-message',

geido · 2024-11-27T10:48:14Z

superset-frontend/src/components/Alert/Alert.stories.tsx

@@ -82,6 +84,7 @@ InteractiveAlert.args = {
  message: smallText,
  description: bigText,
  showIcon: true,
+  closeIcon: <span aria-label="close icon">x</span>,


I think we do not need the closeIcon to be one of the storybook args as it accepts a ReactNode and it is hard to configure this in the storybook UI

geido · 2024-11-27T10:49:06Z

superset-frontend/src/components/Alert/Alert.stories.tsx

@@ -44,6 +44,7 @@ export const AlertGallery = () => (
          type={type}
          showIcon
          closable
+          closeIcon={<span aria-label="close icon">x</span>} // Custom close icon


Maybe remove closeIcon from here and create a separate Alert component with a custom close icon just to show this option in storybook

Thanks for the suggestion! I opted to showcase the closeIcon directly in the existing AlertGallery to keep things simple and avoid adding extra components. Since closeIcon isn't part of the Storybook args, I think this approach still effectively demonstrates its customization without adding another component.

geido · 2024-11-27T10:52:32Z

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

+      closeIcon={closable && <Icons.XSmall aria-label="close icon" />}
+      message={children || 'Default message'}
+      description={description}
+      style={{
        marginBottom: roomBelow ? gridUnit * 4 : 0,
        padding: `${gridUnit * 2}px ${gridUnit * 3}px`,
        alignItems: 'flex-start',
        border: 0,
        backgroundColor: baseColor.light2,


I see you have removed a bunch of custom styles, which is good, but should we bring these in the component theme instead, including also those that you kept? You can see examples here

sadpandajoe · 2024-11-27T18:20:22Z

@LevisNgigi should this PR be a refactor instead of a chore?

LevisNgigi · 2024-11-27T18:30:24Z

@LevisNgigi should this PR be a refactor instead of a chore?

@sadpandajoe Let me change it

geido · 2024-12-02T15:05:17Z

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

+      closeIcon={closable && <Icons.XSmall aria-label="close icon" />}
+      message={children || 'Default message'}
+      description={description}
+      style={{


Do we still need these styles here?

geido · 2024-12-06T17:57:28Z

/testenv up

github-actions · 2024-12-06T17:57:39Z

@geido Processing your ephemeral environment request here.

github-actions · 2024-12-06T18:11:06Z

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

Co-authored-by: Diego Pucci <[email protected]> (cherry picked from commit 79aff68)

LevisNgigi added 3 commits November 25, 2024 16:37

ref:Upgrade Alert component to Ant Design v5

a93ceea

ref:upgrade alert component to antdv5

e11bd21

ref:upgrade Alert component to antdv5

cb0ce6a

pull-request-size bot added the size/M label Nov 26, 2024

ref:upgrade Alert component to antdv5

862683c

LevisNgigi changed the title ~~Upgrade alert antd v5~~ refactor(Alert): Migrate Alert component to Ant Design V5 Nov 26, 2024

ref(tests):fix alert test match to antdv5

3be4770

pull-request-size bot added size/L and removed size/M labels Nov 26, 2024

LevisNgigi changed the title ~~refactor(Alert): Migrate Alert component to Ant Design V5~~ chore(Alert): Migrate Alert component to Ant Design V5 Nov 26, 2024

LevisNgigi and others added 4 commits November 26, 2024 23:24

ref(tests):fix alert class name to antdv5

3b78b20

Merge branch 'apache:master' into upgrade-alert-antd-v5

f80ee39

ref(fix):fix typescript error in props

73d72fb

Merge branch 'upgrade-alert-antd-v5' of github.com:LevisNgigi/superse…

530b29f

…t into upgrade-alert-antd-v5

geido reviewed Nov 27, 2024

View reviewed changes

sadpandajoe added the review:draft label Nov 27, 2024

LevisNgigi changed the title ~~chore(Alert): Migrate Alert component to Ant Design V5~~ Refactor(Alert): Migrate Alert component to Ant Design V5 Nov 27, 2024

LevisNgigi changed the title ~~Refactor(Alert): Migrate Alert component to Ant Design V5~~ refactor(Alert): Migrate Alert component to Ant Design V5 Nov 27, 2024

LevisNgigi added 5 commits November 28, 2024 11:02

ref(update):update class selectors for Alert to antd5

ce178ef

ref(storybook): Separate custom closeIcon into its own component

630904c

fix(storybook): remove custom close icon as its own component

c06adc3

fix(storybook): close icon failing unittests

4093d51

feat(theme): add styling configuration for Alert component

5c3f744

LevisNgigi marked this pull request as ready for review November 30, 2024 12:10

LevisNgigi requested review from jinghua-qa, eschutho, rusackas and betodealmeida as code owners November 30, 2024 12:10

dosubot bot added change:frontend Requires changing the frontend frontend:refactor:antd5 labels Nov 30, 2024

geido added the preset:bounty Issues that have been selected by Preset and have a bounty attached. label Dec 2, 2024

geido reviewed Dec 2, 2024

View reviewed changes

Refactor: Removed redundant styles from index.tsx

0b79f6c

sadpandajoe removed the review:draft label Dec 5, 2024

chore(Alert): Clean up

bab51f1

geido force-pushed the upgrade-alert-antd-v5 branch from 4dda84f to bab51f1 Compare December 6, 2024 13:09

geido approved these changes Dec 6, 2024

View reviewed changes

geido merged commit 79aff68 into apache:master Dec 6, 2024
34 checks passed

betodealmeida pushed a commit that referenced this pull request Jan 14, 2025

refactor(Alert): Migrate Alert component to Ant Design V5 (#31168)

a0c1c2b

Co-authored-by: Diego Pucci <[email protected]> (cherry picked from commit 79aff68)

LevisNgigi deleted the upgrade-alert-antd-v5 branch January 25, 2025 19:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(Alert): Migrate Alert component to Ant Design V5 #31168

refactor(Alert): Migrate Alert component to Ant Design V5 #31168

LevisNgigi commented Nov 26, 2024 •

edited

Loading

geido left a comment

geido Nov 27, 2024

geido Nov 27, 2024

geido Nov 27, 2024

geido Nov 27, 2024

geido Nov 27, 2024

geido Nov 27, 2024

geido Nov 27, 2024

geido Nov 27, 2024

LevisNgigi Nov 28, 2024

geido Nov 27, 2024

sadpandajoe commented Nov 27, 2024

LevisNgigi commented Nov 27, 2024

geido Dec 2, 2024

geido commented Dec 6, 2024

github-actions bot commented Dec 6, 2024

github-actions bot commented Dec 6, 2024

	cy.get('.antd-v5-alert-warning').should('not.exist');
	cy.get('.antd5-alert-warning').should('not.exist');

	alertMessage: '.antd-v5-alert-message',
	alertMessage: '.antd5-alert-message',

	alertXUnsavedFilters: '.antd-v5-alert-message',
	alertXUnsavedFilters: '.antd5-alert-message',

refactor(Alert): Migrate Alert component to Ant Design V5 #31168

refactor(Alert): Migrate Alert component to Ant Design V5 #31168

Conversation

LevisNgigi commented Nov 26, 2024 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before

After

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

geido left a 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

sadpandajoe commented Nov 27, 2024

LevisNgigi commented Nov 27, 2024

Choose a reason for hiding this comment

geido commented Dec 6, 2024

github-actions bot commented Dec 6, 2024

github-actions bot commented Dec 6, 2024

LevisNgigi commented Nov 26, 2024 •

edited

Loading