From 070f0b6cb2a1c3385bc2a7ed2448aa7eb1251666 Mon Sep 17 00:00:00 2001 From: Phillip Kelley-Dotson Date: Wed, 14 Jul 2021 11:04:21 -0700 Subject: [PATCH] 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 --- .../integration/dashboard/edit_mode.test.js | 4 +-- .../dashboard/edit_properties.test.ts | 2 +- .../integration/dashboard/markdown.test.ts | 2 +- .../integration/dashboard/save.test.js | 6 ++-- .../src/components/IconButton/index.tsx | 22 +++++++----- .../src/dashboard/components/Header/index.jsx | 7 ++-- .../database/DatabaseModal/index.test.jsx | 34 +++++++++---------- 7 files changed, 43 insertions(+), 34 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.test.js b/superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.test.js index 62f40c616..d799dccd3 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.test.js +++ b/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'); }); }); }); diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/edit_properties.test.ts b/superset-frontend/cypress-base/cypress/integration/dashboard/edit_properties.test.ts index ad20010a8..c5cbfb7bb 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/edit_properties.test.ts +++ b/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(); diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/markdown.test.ts b/superset-frontend/cypress-base/cypress/integration/dashboard/markdown.test.ts index ba5670638..dfde7bf45 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/markdown.test.ts +++ b/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 diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js b/superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js index b52ff806f..2555730c3 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/save.test.js +++ b/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 diff --git a/superset-frontend/src/components/IconButton/index.tsx b/superset-frontend/src/components/IconButton/index.tsx index d36015e29..0d80911bf 100644 --- a/superset-frontend/src/components/IconButton/index.tsx +++ b/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( {icon && {altText}} {!icon && ( - )} diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index 6b6b37840..b4595774f 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/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} > - + )} diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx index eb6722f03..6214e997b 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.jsx +++ b/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 ---------- // - 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', () => { }); // - 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() =>