geido commented on code in PR #31168:
URL: https://github.com/apache/superset/pull/31168#discussion_r1860434519
##########
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',
Review Comment:
```suggestion
infoAlert: '.antd5-alert',
```
##########
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
Review Comment:
Maybe remove `closeIcon` from here and create a separate Alert component
with a custom close icon just to show this option in storybook
##########
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');
Review Comment:
```suggestion
cy.get('.antd5-alert-warning').should('not.exist');
```
##########
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>,
Review Comment:
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
##########
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',
Review Comment:
```suggestion
alertMessage: '.antd5-alert-message',
```
##########
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');
Review Comment:
```suggestion
cy.get('.antd5-alert-warning').should('not.exist');
```
##########
superset-frontend/src/components/Alert/index.tsx:
##########
@@ -56,32 +58,18 @@ export default function Alert(props: AlertProps) {
<AntdAlert
role="alert"
showIcon={showIcon}
- icon={<AlertIcon aria-label={`${type} icon`} />}
- closeText={closable && <Icons.XSmall aria-label="close icon" />}
- css={{
+ icon={showIcon && <AlertIcon aria-label={`${type} icon`} />}
+ 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,
Review Comment:
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](https://github.com/apache/superset/pull/29786/files#diff-7c635266fc6affa5c3d678e1d8cf2a89451b283e168a1fb1f5b878342adb4a5aR72)
##########
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',
Review Comment:
```suggestion
alertXUnsavedFilters: '.antd5-alert-message',
```
##########
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',
Review Comment:
```suggestion
alertMessage: '.antd5-alert-message',
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
