diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx index a9a3d1bdaf7..801363e51c7 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx @@ -28,7 +28,7 @@ import { getColumnTypeTooltipNode, } from './labelUtils'; import { SQLPopover } from './SQLPopover'; -import InfoTooltipWithTrigger from './InfoTooltipWithTrigger'; +import { InfoTooltipWithTrigger } from './InfoTooltipWithTrigger'; export type ColumnOptionProps = { column: ColumnMeta; @@ -99,14 +99,12 @@ export function ColumnOption({ )} {warningMarkdown && ( } label={`warn-${column.column_name}`} - iconsStyle={{ marginLeft: 0 }} + iconStyle={{ marginLeft: 0 }} {...(column.error_text && { - className: 'text-danger', - icon: 'exclamation-circle', + type: 'error', })} /> )} diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/ControlHeader.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/ControlHeader.tsx index 44e47d55e99..0172b5656a5 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/ControlHeader.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/ControlHeader.tsx @@ -74,7 +74,7 @@ export function ControlHeader({ label={t('bolt')} tooltip={t('Changing this control takes effect instantly')} placement="top" - icon="bolt" + type="notice" />{' '} )} diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/InfoTooltipWithTrigger.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/InfoTooltipWithTrigger.tsx index 5551e94a809..d97b974ae6f 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/InfoTooltipWithTrigger.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/InfoTooltipWithTrigger.tsx @@ -16,65 +16,111 @@ * specific language governing permissions and limitations * under the License. */ -import { CSSProperties } from 'react'; +import { KeyboardEvent, useMemo } from 'react'; +import { SerializedStyles, CSSObject } from '@emotion/react'; import { kebabCase } from 'lodash'; -import { t } from '@superset-ui/core'; +import { css, t, useTheme, themeObject } from '@superset-ui/core'; +import { + CloseCircleOutlined, + InfoCircleOutlined, + WarningOutlined, + ThunderboltOutlined, + QuestionCircleOutlined, +} from '@ant-design/icons'; +import { Button } from 'antd-v5'; import { Tooltip, TooltipProps, TooltipPlacement } from './Tooltip'; export interface InfoTooltipWithTriggerProps { label?: string; tooltip?: TooltipProps['title']; - icon?: string; onClick?: () => void; placement?: TooltipPlacement; - bsStyle?: string; className?: string; - iconsStyle?: CSSProperties; + iconStyle?: CSSObject | SerializedStyles; + type?: 'info' | 'warning' | 'notice' | 'error' | 'question'; + iconSize?: 'xs' | 's' | 'm' | 'l' | 'xl' | 'xxl'; } -export function InfoTooltipWithTrigger({ +export const InfoTooltipWithTrigger = ({ + type = 'info', + iconSize = 's', label, tooltip, - bsStyle, onClick, - icon = 'info-circle', className = 'text-muted', placement = 'right', - iconsStyle = {}, -}: InfoTooltipWithTriggerProps) { - const iconClass = `fa fa-${icon} ${className} ${ - bsStyle ? `text-${bsStyle}` : '' - }`; - const iconEl = ( - { - if (event.key === 'Enter' || event.key === ' ') { - onClick(); - } - }) - } - /> + iconStyle, +}: InfoTooltipWithTriggerProps) => { + const theme = useTheme(); + + const infoTooltipWithTriggerVariants = useMemo( + () => ({ + info: { color: theme.colorIcon, icon: }, + question: { color: theme.colorIcon, icon: }, + warning: { color: theme.colorWarning, icon: }, + notice: { color: theme.colorWarning, icon: }, + error: { color: theme.colorError, icon: }, + }), + [theme], ); + + const variant = type ? infoTooltipWithTriggerVariants[type] : null; + + const iconCss = css` + color: ${variant?.color ?? theme.colorIcon}; + font-size: ${themeObject.getFontSize(iconSize)}px; + `; + + const handleKeyDown = (event: KeyboardEvent) => { + if (onClick && (event.key === 'Enter' || event.key === ' ')) { + onClick(); + } + }; + + const iconEl = ( + + ); + if (!tooltip) { return iconEl; } + return ( {iconEl} ); -} - -export default InfoTooltipWithTrigger; +}; diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx index 81f0f97f48c..d3f2df4c885 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx @@ -25,7 +25,7 @@ import { SafeMarkdown, SupersetTheme, } from '@superset-ui/core'; -import InfoTooltipWithTrigger from './InfoTooltipWithTrigger'; +import { InfoTooltipWithTrigger } from './InfoTooltipWithTrigger'; import { ColumnTypeLabel } from './ColumnTypeLabel/ColumnTypeLabel'; import CertifiedIconWithTooltip from './CertifiedIconWithTooltip'; import Tooltip from './Tooltip'; @@ -112,14 +112,12 @@ export function MetricOption({ )} {warningMarkdown && ( } label={`warn-${metric.metric_name}`} - iconsStyle={{ marginLeft: 0 }} + iconStyle={{ marginLeft: 0 }} {...(metric.error_text && { - className: 'text-danger', - icon: 'exclamation-circle', + type: 'error', })} /> )} diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/Tooltip.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/Tooltip.tsx index 73f8f45a781..5551e1994c4 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/Tooltip.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/Tooltip.tsx @@ -45,8 +45,6 @@ export const Tooltip = ({ ...overlayStyle, }, }} - // make the tooltip display closer to the label - align={{ offset: [0, 1] }} color={defaultColor || color} trigger="hover" placement="bottom" diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/components/ColumnOption.test.tsx b/superset-frontend/packages/superset-ui-chart-controls/test/components/ColumnOption.test.tsx index c5b369aa2db..68c6f626330 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/components/ColumnOption.test.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/test/components/ColumnOption.test.tsx @@ -34,9 +34,12 @@ jest.mock('../../src/components/ColumnTypeLabel/ColumnTypeLabel', () => ({
{type}
), })); -jest.mock('../../src/components/InfoTooltipWithTrigger', () => () => ( -
-)); + +jest.mock('../../src/components/InfoTooltipWithTrigger', () => ({ + InfoTooltipWithTrigger: () => ( +
+ ), +})); const defaultProps: ColumnOptionProps = { column: { diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/components/InfoTooltipWithTrigger.test.tsx b/superset-frontend/packages/superset-ui-chart-controls/test/components/InfoTooltipWithTrigger.test.tsx index 0011f862b29..d243b1fd9f8 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/components/InfoTooltipWithTrigger.test.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/test/components/InfoTooltipWithTrigger.test.tsx @@ -44,31 +44,29 @@ test('renders a tooltip', () => { expect(getAllByTestId('mock-tooltip').length).toEqual(1); }); -test('renders an info icon', () => { - const { container } = setup(); - expect(container.getElementsByClassName('fa-info-circle')).toHaveLength(1); -}); - -test('responds to keypresses', () => { +test('responds to keydown events', () => { const clickHandler = jest.fn(); const { getByRole } = setup({ label: 'test', tooltip: 'this is a test', onClick: clickHandler, }); - fireEvent.keyPress(getByRole('button'), { + + fireEvent.keyDown(getByRole('button'), { key: 'Tab', code: 9, charCode: 9, }); expect(clickHandler).toHaveBeenCalledTimes(0); - fireEvent.keyPress(getByRole('button'), { + + fireEvent.keyDown(getByRole('button'), { key: 'Enter', code: 13, charCode: 13, }); expect(clickHandler).toHaveBeenCalledTimes(1); - fireEvent.keyPress(getByRole('button'), { + + fireEvent.keyDown(getByRole('button'), { key: ' ', code: 32, charCode: 32, @@ -76,9 +74,47 @@ test('responds to keypresses', () => { expect(clickHandler).toHaveBeenCalledTimes(2); }); -test('has a bsStyle', () => { +test('finds the info circle icon inside info variant', () => { const { container } = setup({ - bsStyle: 'something', + type: 'info', }); - expect(container.getElementsByClassName('text-something')).toHaveLength(1); + + const iconSpan = container.querySelector('svg[data-icon="info-circle"]'); + expect(iconSpan).toBeInTheDocument(); +}); + +test('finds the warning icon inside warning variant', () => { + const { container } = setup({ + type: 'warning', + }); + + const iconSpan = container.querySelector('svg[data-icon="warning"]'); + expect(iconSpan).toBeInTheDocument(); +}); + +test('finds the close circle icon inside error variant', () => { + const { container } = setup({ + type: 'error', + }); + + const iconSpan = container.querySelector('svg[data-icon="close-circle"]'); + expect(iconSpan).toBeInTheDocument(); +}); + +test('finds the question circle icon inside question variant', () => { + const { container } = setup({ + type: 'question', + }); + + const iconSpan = container.querySelector('svg[data-icon="question-circle"]'); + expect(iconSpan).toBeInTheDocument(); +}); + +test('finds the thunderbolt icon inside notice variant', () => { + const { container } = setup({ + type: 'notice', + }); + + const iconSpan = container.querySelector('svg[data-icon="thunderbolt"]'); + expect(iconSpan).toBeInTheDocument(); }); diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/components/MetricOption.test.tsx b/superset-frontend/packages/superset-ui-chart-controls/test/components/MetricOption.test.tsx index 49b78159f37..03da5df2ffc 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/components/MetricOption.test.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/test/components/MetricOption.test.tsx @@ -21,9 +21,12 @@ import { render } from '@testing-library/react'; import { ThemeProvider, supersetTheme } from '@superset-ui/core'; import { MetricOption, MetricOptionProps } from '../../src'; -jest.mock('../../src/components/InfoTooltipWithTrigger', () => () => ( -
-)); +jest.mock('../../src/components/InfoTooltipWithTrigger', () => ({ + InfoTooltipWithTrigger: () => ( +
+ ), +})); + jest.mock('../../src/components/ColumnTypeLabel/ColumnTypeLabel', () => ({ ColumnTypeLabel: () =>
, })); diff --git a/superset-frontend/plugins/legacy-plugin-chart-partition/src/OptionDescription.tsx b/superset-frontend/plugins/legacy-plugin-chart-partition/src/OptionDescription.tsx index 7a45cbea5ec..d4e68b784ec 100644 --- a/superset-frontend/plugins/legacy-plugin-chart-partition/src/OptionDescription.tsx +++ b/superset-frontend/plugins/legacy-plugin-chart-partition/src/OptionDescription.tsx @@ -36,8 +36,8 @@ export default function OptionDescription({ option }: { option: ColumnMeta }) { {option.label} {option.description && ( diff --git a/superset-frontend/plugins/plugin-chart-handlebars/src/plugin/controls/handlebarTemplate.tsx b/superset-frontend/plugins/plugin-chart-handlebars/src/plugin/controls/handlebarTemplate.tsx index 6eef1eb65a2..a15b8936231 100644 --- a/superset-frontend/plugins/plugin-chart-handlebars/src/plugin/controls/handlebarTemplate.tsx +++ b/superset-frontend/plugins/plugin-chart-handlebars/src/plugin/controls/handlebarTemplate.tsx @@ -71,7 +71,7 @@ ${helperDescriptions
{props.label} } />
diff --git a/superset-frontend/plugins/plugin-chart-handlebars/src/plugin/controls/style.tsx b/superset-frontend/plugins/plugin-chart-handlebars/src/plugin/controls/style.tsx index d15fc7d961d..09aee379db5 100644 --- a/superset-frontend/plugins/plugin-chart-handlebars/src/plugin/controls/style.tsx +++ b/superset-frontend/plugins/plugin-chart-handlebars/src/plugin/controls/style.tsx @@ -48,7 +48,7 @@ const StyleControl = (props: CustomControlConfig) => {
{props.label}
diff --git a/superset-frontend/src/SqlLab/components/ExploreCtasResultsButton/index.tsx b/superset-frontend/src/SqlLab/components/ExploreCtasResultsButton/index.tsx index 98fb012e5d2..62a9e6b0d5a 100644 --- a/superset-frontend/src/SqlLab/components/ExploreCtasResultsButton/index.tsx +++ b/superset-frontend/src/SqlLab/components/ExploreCtasResultsButton/index.tsx @@ -23,8 +23,8 @@ import { addInfoToast, addDangerToast, } from 'src/SqlLab/actions/sqlLab'; -import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; -import { Button } from 'src/components'; +import { Button, IconTooltip } from 'src/components'; +import { Icons } from 'src/components/Icons'; import { exploreChart } from 'src/explore/exploreUtils'; import { SqlLabRootState } from 'src/SqlLab/types'; @@ -82,11 +82,8 @@ const ExploreCtasResultsButton = ({ onClick={visualize} tooltip={t('Explore the result set in the data exploration view')} > - {' '} + + {t('Explore')} ); diff --git a/superset-frontend/src/SqlLab/components/TemplateParamsEditor/index.tsx b/superset-frontend/src/SqlLab/components/TemplateParamsEditor/index.tsx index 8079c8e417d..d23290ea4e6 100644 --- a/superset-frontend/src/SqlLab/components/TemplateParamsEditor/index.tsx +++ b/superset-frontend/src/SqlLab/components/TemplateParamsEditor/index.tsx @@ -20,7 +20,6 @@ import { useState, useEffect } from 'react'; import { t, styled } from '@superset-ui/core'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; import { debounce } from 'lodash'; - import { Badge, ConfigEditor, Tooltip } from 'src/components'; import ModalTrigger from 'src/components/ModalTrigger'; import { FAST_DEBOUNCE } from 'src/constants'; @@ -107,8 +106,7 @@ const TemplateParamsEditor = ({ {!isValid && ( diff --git a/superset-frontend/src/components/IconTooltip/IconTooltip.stories.tsx b/superset-frontend/src/components/IconTooltip/IconTooltip.stories.tsx index b9ecf894f2c..ca955071f40 100644 --- a/superset-frontend/src/components/IconTooltip/IconTooltip.stories.tsx +++ b/superset-frontend/src/components/IconTooltip/IconTooltip.stories.tsx @@ -40,19 +40,20 @@ const PLACEMENTS = [ 'topRight', ]; -const theme = useTheme(); - -export const InteractiveIconTooltip = (args: IconTooltipProps) => ( -
- - - -
-); +export const InteractiveIconTooltip = (args: IconTooltipProps) => { + const theme = useTheme(); + return ( +
+ + + +
+ ); +}; InteractiveIconTooltip.args = { tooltip: 'Tooltip', diff --git a/superset-frontend/src/components/Icons/Icons.stories.tsx b/superset-frontend/src/components/Icons/Icons.stories.tsx index 496817df6e8..fa45056fdb1 100644 --- a/superset-frontend/src/components/Icons/Icons.stories.tsx +++ b/superset-frontend/src/components/Icons/Icons.stories.tsx @@ -18,7 +18,7 @@ */ import { useState } from 'react'; import { styled, supersetTheme } from '@superset-ui/core'; -import { Input } from 'antd-v5'; +import { Input } from '../Input'; import { Icons, IconNameType } from '.'; import type { IconType } from './types'; import { BaseIconComponent } from './BaseIcon'; diff --git a/superset-frontend/src/components/PopoverSection/index.tsx b/superset-frontend/src/components/PopoverSection/index.tsx index 7f3acbb78c1..be201a24b09 100644 --- a/superset-frontend/src/components/PopoverSection/index.tsx +++ b/superset-frontend/src/components/PopoverSection/index.tsx @@ -63,7 +63,7 @@ export default function PopoverSection({ margin-right: ${theme.sizeUnit}px; `} > - { test('enables the "Save Changes" button', async () => { setup(); - const allowedDomainsInput = await screen.findByLabelText( - new RegExp(/Allowed Domains/, 'i'), - ); + const allowedDomainsInput = await screen.findByRole('textbox', { + name: /Allowed Domains/i, + }); + const saveChangesBtn = screen.getByRole('button', { name: 'Save changes' }); expect(saveChangesBtn).toBeDisabled(); diff --git a/superset-frontend/src/dashboard/components/gridComponents/Row.jsx b/superset-frontend/src/dashboard/components/gridComponents/Row.jsx index b38907dbffb..6e4cb5cecf8 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Row.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Row.jsx @@ -288,7 +288,7 @@ const Row = props => { } + icon={} /> )} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx index 49731a649bf..4f993d0158a 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx @@ -126,12 +126,12 @@ const TIME_GRAIN_REGEX = /^time grain$/i; const FILTER_SETTINGS_REGEX = /^filter settings$/i; const DEFAULT_VALUE_REGEX = /^filter has default value$/i; const MULTIPLE_REGEX = /^can select multiple values$/i; -const REQUIRED_REGEX = /^filter value is required$/i; +const FILTER_REQUIRED_REGEX = /^filter value is required/i; const DEPENDENCIES_REGEX = /^values are dependent on other filters$/i; -const FIRST_VALUE_REGEX = /^select first filter value by default$/i; -const INVERSE_SELECTION_REGEX = /^inverse selection$/i; -const SEARCH_ALL_REGEX = /^dynamically search all filter values$/i; -const PRE_FILTER_REGEX = /^pre-filter available values$/i; +const FIRST_VALUE_REGEX = /^select first filter value by default/i; +const INVERSE_SELECTION_REGEX = /^inverse selection/i; +const SEARCH_ALL_REGEX = /^dynamically search all filter values/i; +const PRE_FILTER_REGEX = /^pre-filter available values/i; const SORT_REGEX = /^sort filter values$/i; const SAVE_REGEX = /^save$/i; const NAME_REQUIRED_REGEX = /^name is required$/i; @@ -181,7 +181,7 @@ test('renders a value filter type', () => { expect(screen.getByText(COLUMN_REGEX)).toBeInTheDocument(); expect(getCheckbox(DEFAULT_VALUE_REGEX)).not.toBeChecked(); - expect(getCheckbox(REQUIRED_REGEX)).not.toBeChecked(); + expect(getCheckbox(FILTER_REQUIRED_REGEX)).not.toBeChecked(); expect(queryCheckbox(DEPENDENCIES_REGEX)).not.toBeInTheDocument(); expect(getCheckbox(FIRST_VALUE_REGEX)).not.toBeChecked(); expect(getCheckbox(INVERSE_SELECTION_REGEX)).not.toBeChecked(); @@ -203,7 +203,7 @@ test('renders a numerical range filter type', async () => { expect(screen.getByText(FILTER_NAME_REGEX)).toBeInTheDocument(); expect(screen.getByText(DATASET_REGEX)).toBeInTheDocument(); expect(screen.getByText(COLUMN_REGEX)).toBeInTheDocument(); - expect(screen.getByText(REQUIRED_REGEX)).toBeInTheDocument(); + expect(screen.getByText(FILTER_REQUIRED_REGEX)).toBeInTheDocument(); expect(getCheckbox(DEFAULT_VALUE_REGEX)).not.toBeChecked(); expect(getCheckbox(PRE_FILTER_REGEX)).not.toBeChecked(); diff --git a/superset-frontend/src/explore/components/ControlHeader.tsx b/superset-frontend/src/explore/components/ControlHeader.tsx index 8eeeaafcafe..57c36e23b30 100644 --- a/superset-frontend/src/explore/components/ControlHeader.tsx +++ b/superset-frontend/src/explore/components/ControlHeader.tsx @@ -121,7 +121,7 @@ const ControlHeader: FC = ({ label={t('bolt')} tooltip={t('Changing this control takes effect instantly')} placement="top" - icon="bolt" + type="notice" />{' '} )} diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.tsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.tsx index 7f079d57170..48f639539e5 100644 --- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.tsx +++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.tsx @@ -192,7 +192,7 @@ class AnnotationLayerControl extends PureComponent { return ( ); diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/Option.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/Option.tsx index 41d66a159b5..86aef9fd61f 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/Option.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/Option.tsx @@ -72,9 +72,8 @@ export default function Option({ {(!!datasourceWarningMessage || isExtra) && ( {(!!datasourceWarningMessage || isExtra) && (