refactor: icon to icons for IconButton and Header component (#15647)

* initial commit

* fix test

* last one

* fix cypress

* remove gridunit for fonsize

* fix cypress

* more data-test removal

* last one

* more data-test

superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.test.js

@@ -23,7 +23,7 @@ describe('Dashboard edit mode', () => {
     cy.login();
     cy.visit(WORLD_HEALTH_DASHBOARD);
     cy.get('[data-test="dashboard-header"]')
-      .find('[data-test=edit-alt]')
+      .find('[aria-label=edit-alt]')
       .click();
   });
 
@@ -96,7 +96,7 @@ describe('Dashboard edit mode', () => {
       .click();
     cy.get('[data-test="dashboard-header"]').within(() => {
       cy.get('[data-test="dashboard-edit-actions"]').should('not.be.visible');
-      cy.get('[data-test="edit-alt"]').should('be.visible');
+      cy.get('[aria-label="edit-alt"]').should('be.visible');
     });
   });
 });

superset-frontend/cypress-base/cypress/integration/dashboard/edit_properties.test.ts

@@ -77,7 +77,7 @@ describe('Dashboard edit action', () => {
     cy.get('.dashboard-grid', { timeout: 50000 })
       .should('be.visible') // wait for 50 secs to load dashboard
       .then(() => {
-        cy.get('.dashboard-header [data-test=edit-alt]')
+        cy.get('.dashboard-header [aria-label=edit-alt]')
           .should('be.visible')
           .click();
         openDashboardEditProperties();

superset-frontend/cypress-base/cypress/integration/dashboard/markdown.test.ts

@@ -30,7 +30,7 @@ describe('Dashboard edit markdown', () => {
       numScripts = nodes.length;
     });
     cy.get('[data-test="dashboard-header"]')
-      .find('[data-test="edit-alt"]')
+      .find('[aria-label="edit-alt"]')
       .click();
 
     // lazy load - need to open dropdown for the scripts to load

superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js

@@ -25,7 +25,7 @@ import {
 } from './dashboard.helper';
 
 function openDashboardEditProperties() {
-  cy.get('.dashboard-header [data-test=edit-alt]').click();
+  cy.get('.dashboard-header [aria-label=edit-alt]').click();
   cy.get('#save-dash-split-button').trigger('click', { force: true });
   cy.get('.dropdown-menu').contains('Edit dashboard properties').click();
 }
@@ -68,7 +68,7 @@ describe('Dashboard save action', () => {
     WORLD_HEALTH_CHARTS.forEach(waitForChartLoad);
 
     // remove box_plot chart from dashboard
-    cy.get('[data-test="edit-alt"]').click({ timeout: 5000 });
+    cy.get('[aria-label="edit-alt"]').click({ timeout: 5000 });
     cy.get('[data-test="dashboard-delete-component-button"]')
       .last()
       .trigger('moustenter')
@@ -87,7 +87,7 @@ describe('Dashboard save action', () => {
     // go back to view mode
     cy.wait('@saveRequest');
     cy.get('[data-test="dashboard-header"]')
-      .find('[data-test="edit-alt"]')
+      .find('[aria-label="edit-alt"]')
       .click();
 
     // deleted boxplot should still not exist

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

@@ -17,10 +17,10 @@
  * under the License.
  */
 import React from 'react';
-import { styled, supersetTheme } from '@superset-ui/core';
+import { styled } from '@superset-ui/core';
 import Button from 'src/components/Button';
 import { ButtonProps as AntdButtonProps } from 'antd/lib/button';
-import Icon from 'src/components/Icon';
+import Icons from 'src/components/Icons';
 import LinesEllipsis from 'react-lines-ellipsis';
 
 export interface IconButtonProps extends AntdButtonProps {
@@ -41,6 +41,15 @@ const StyledImage = styled.div`
   height: ${({ theme }) => theme.gridUnit * 18}px;
   margin: ${({ theme }) => theme.gridUnit * 3}px 0;
 
+  .default-db-icon {
+    font-size: 36px;
+    color: ${({ theme }) => theme.colors.grayscale.base};
+    margin-right: 0;
+    span:first-of-type {
+      margin-right: 0;
+    }
+  }
+
   &:first-of-type {
     margin-right: 0;
   }
@@ -97,12 +106,9 @@ const IconButton = styled(
       <StyledImage>
         {icon && <img src={icon} alt={altText} />}
         {!icon && (
-          <Icon
-            color={supersetTheme.colors.primary.base}
-            height="40"
-            width="40"
-            viewBox="0 0 18 18"
-            name="default-database"
+          <Icons.DatabaseOutlined
+            className="default-db-icon"
+            aria-label="default-icon"
           />
         )}
       </StyledImage>

superset-frontend/src/dashboard/components/Header/index.jsx

@@ -29,7 +29,7 @@ import {
   LOG_ACTIONS_TOGGLE_EDIT_DASHBOARD,
 } from 'src/logger/LogUtils';
 
-import Icon from 'src/components/Icon';
+import Icons from 'src/components/Icons';
 import Button from 'src/components/Button';
 import EditableTitle from 'src/components/EditableTitle';
 import FaveStar from 'src/components/FaveStar';
@@ -110,6 +110,9 @@ const StyledDashboardHeader = styled.div`
   padding: 0 ${({ theme }) => theme.gridUnit * 6}px;
   border-bottom: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
 
+  .action-button > span {
+    color: ${({ theme }) => theme.colors.grayscale.base};
+  }
   button,
   .fave-unfave-icon {
     margin-left: ${({ theme }) => theme.gridUnit * 2}px;
@@ -491,7 +494,7 @@ class Header extends React.PureComponent {
                 className="action-button"
                 onClick={this.toggleEditMode}
               >
-                <Icon name="edit-alt" />
+                <Icons.EditAlt />
               </span>
             </>
           )}

superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx

@@ -216,7 +216,7 @@ describe('DatabaseModal', () => {
     it('renders the initial load of Step 1 correctly', async () => {
       // ---------- Components ----------
       // <TabHeader> - AntD header
-      const closeButton = screen.getByRole('button', { name: /close/i });
+      const closeButton = screen.getByLabelText('Close');
       const step1Header = screen.getByRole('heading', {
         name: /connect a database/i,
       });
@@ -227,25 +227,25 @@ describe('DatabaseModal', () => {
       });
       // <IconButton> - Preferred database buttons
       const preferredDbButtonPostgreSQL = screen.getByRole('button', {
-        name: /default-database postgresql/i,
+        name: /postgresql/i,
       });
       const preferredDbTextPostgreSQL = within(
         preferredDbButtonPostgreSQL,
       ).getByText(/postgresql/i);
       const preferredDbButtonPresto = screen.getByRole('button', {
-        name: /default-database presto/i,
+        name: /presto/i,
       });
       const preferredDbTextPresto = within(preferredDbButtonPresto).getByText(
         /presto/i,
       );
       const preferredDbButtonMySQL = screen.getByRole('button', {
-        name: /default-database mysql/i,
+        name: /mysql/i,
       });
       const preferredDbTextMySQL = within(preferredDbButtonMySQL).getByText(
         /mysql/i,
       );
       const preferredDbButtonSQLite = screen.getByRole('button', {
-        name: /default-database sqlite/i,
+        name: /sqlite/i,
       });
       const preferredDbTextSQLite = within(preferredDbButtonSQLite).getByText(
         /sqlite/i,
@@ -253,7 +253,7 @@ describe('DatabaseModal', () => {
       // All dbs render with this icon in this testing environment,
       // The Icon count should equal the count of databases rendered
       const preferredDbIcon = screen.getAllByRole('img', {
-        name: /default-database/i,
+        name: /default-icon/i,
       });
       // renderAvailableSelector() => <Select> - Supported databases selector
       const supportedDbsHeader = screen.getByRole('heading', {
@@ -315,7 +315,7 @@ describe('DatabaseModal', () => {
       // On step 1, click dbButton to access SQL Alchemy form
       userEvent.click(
         screen.getByRole('button', {
-          name: /default-database sqlite/i,
+          name: /sqlite/i,
         }),
       );
 
@@ -400,7 +400,7 @@ describe('DatabaseModal', () => {
       // On step 1, click dbButton to access step 2
       userEvent.click(
         screen.getByRole('button', {
-          name: /default-database sqlite/i,
+          name: /sqlite/i,
         }),
       );
       // Click the "Advanced" tab
@@ -499,7 +499,7 @@ describe('DatabaseModal', () => {
       // On step 1, click dbButton to access step 2
       userEvent.click(
         screen.getByRole('button', {
-          name: /default-database sqlite/i,
+          name: /sqlite/i,
         }),
       );
       // Click the "Advanced" tab
@@ -649,7 +649,7 @@ describe('DatabaseModal', () => {
       // On step 1, click dbButton to access step 2
       userEvent.click(
         screen.getByRole('button', {
-          name: /default-database sqlite/i,
+          name: /sqlite/i,
         }),
       );
       // Click the "Advanced" tab
@@ -711,7 +711,7 @@ describe('DatabaseModal', () => {
       // On step 1, click dbButton to access step 2
       userEvent.click(
         screen.getByRole('button', {
-          name: /default-database sqlite/i,
+          name: /sqlite/i,
         }),
       );
       // Click the "Advanced" tab
@@ -777,7 +777,7 @@ describe('DatabaseModal', () => {
       // On step 1, click dbButton to access step 2
       userEvent.click(
         screen.getByRole('button', {
-          name: /default-database sqlite/i,
+          name: /sqlite/i,
         }),
       );
       // Click the "Advanced" tab
@@ -847,7 +847,7 @@ describe('DatabaseModal', () => {
       // On step 1, click dbButton to access step 2
       userEvent.click(
         screen.getByRole('button', {
-          name: /default-database postgresql/i,
+          name: /postgresql/i,
         }),
       );
 
@@ -860,7 +860,7 @@ describe('DatabaseModal', () => {
       // ---------- Dynamic example (3-step form)
       // Click the PostgreSQL button to enter the dynamic form
       const postgreSQLButton = screen.getByRole('button', {
-        name: /default-database postgresql/i,
+        name: /postgresql/i,
       });
       userEvent.click(postgreSQLButton);
 
@@ -876,7 +876,7 @@ describe('DatabaseModal', () => {
       userEvent.click(backButton);
 
       const sqliteButton = screen.getByRole('button', {
-        name: /default-database sqlite/i,
+        name: /sqlite/i,
       });
       userEvent.click(sqliteButton);
 
@@ -890,7 +890,7 @@ describe('DatabaseModal', () => {
       beforeEach(() => {
         userEvent.click(
           screen.getByRole('button', {
-            name: /default-database sqlite/i,
+            name: /sqlite/i,
           }),
         );
       });
@@ -959,7 +959,7 @@ describe('DatabaseModal', () => {
       beforeEach(() => {
         userEvent.click(
           screen.getByRole('button', {
-            name: /default-database postgresql/i,
+            name: /postgresql/i,
           }),
         );
       });