refactor(InfoTooltipWithTrigger): Replace support for fa icons with antd5 icons (#33256)

- Define and centralized tooltip icon variants (info, warning, notice, error, question)
- Replace old FontAwesome icons
- Update tests to reflect the new icons and variant behavior
This commit is contained in:
Enzo Martellucci
2025-05-05 18:07:15 +02:00
committed by GitHub
parent 49bcf79f71
commit 9da62ed63a
23 changed files with 190 additions and 113 deletions

View File

@@ -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 && (
<InfoTooltipWithTrigger
className="text-warning"
icon="warning"
type="warning"
tooltip={<SafeMarkdown source={warningMarkdown} />}
label={`warn-${column.column_name}`}
iconsStyle={{ marginLeft: 0 }}
iconStyle={{ marginLeft: 0 }}
{...(column.error_text && {
className: 'text-danger',
icon: 'exclamation-circle',
type: 'error',
})}
/>
)}

View File

@@ -74,7 +74,7 @@ export function ControlHeader({
label={t('bolt')}
tooltip={t('Changing this control takes effect instantly')}
placement="top"
icon="bolt"
type="notice"
/>{' '}
</span>
)}

View File

@@ -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 = (
<i
role="button"
aria-label={t('Show info tooltip')}
tabIndex={0}
className={iconClass}
style={{ cursor: onClick ? 'pointer' : undefined, ...iconsStyle }}
onClick={onClick}
onKeyPress={
onClick &&
(event => {
if (event.key === 'Enter' || event.key === ' ') {
onClick();
}
})
}
/>
iconStyle,
}: InfoTooltipWithTriggerProps) => {
const theme = useTheme();
const infoTooltipWithTriggerVariants = useMemo(
() => ({
info: { color: theme.colorIcon, icon: <InfoCircleOutlined /> },
question: { color: theme.colorIcon, icon: <QuestionCircleOutlined /> },
warning: { color: theme.colorWarning, icon: <WarningOutlined /> },
notice: { color: theme.colorWarning, icon: <ThunderboltOutlined /> },
error: { color: theme.colorError, icon: <CloseCircleOutlined /> },
}),
[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<HTMLSpanElement>) => {
if (onClick && (event.key === 'Enter' || event.key === ' ')) {
onClick();
}
};
const iconEl = (
<Button
type="text"
variant="text"
aria-label={t('Show info tooltip')}
className={className}
css={[
theme => css`
vertical-align: text-bottom;
box-shadow: none;
padding: 0;
height: auto;
background: none;
&&&:hover,
&&&:focus,
&&&:active {
box-shadow: none;
background: none;
outline: none;
color: ${theme.colorTextTertiary};
}
${onClick ? 'cursor: pointer;' : ''}
`,
iconCss,
iconStyle,
]}
onClick={onClick}
onKeyDown={handleKeyDown}
>
{variant?.icon}
</Button>
);
if (!tooltip) {
return iconEl;
}
return (
<Tooltip
id={`${kebabCase(label)}-tooltip`}
id={`${kebabCase(label) || Math.floor(Math.random() * 10000)}-tooltip`}
title={tooltip}
placement={placement}
>
{iconEl}
</Tooltip>
);
}
export default InfoTooltipWithTrigger;
};

View File

@@ -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 && (
<InfoTooltipWithTrigger
className="text-warning"
icon="warning"
type="warning"
tooltip={<SafeMarkdown source={warningMarkdown} />}
label={`warn-${metric.metric_name}`}
iconsStyle={{ marginLeft: 0 }}
iconStyle={{ marginLeft: 0 }}
{...(metric.error_text && {
className: 'text-danger',
icon: 'exclamation-circle',
type: 'error',
})}
/>
)}

View File

@@ -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"

View File

@@ -34,9 +34,12 @@ jest.mock('../../src/components/ColumnTypeLabel/ColumnTypeLabel', () => ({
<div data-test="mock-column-type-label">{type}</div>
),
}));
jest.mock('../../src/components/InfoTooltipWithTrigger', () => () => (
<div data-test="mock-info-tooltip-with-trigger" />
));
jest.mock('../../src/components/InfoTooltipWithTrigger', () => ({
InfoTooltipWithTrigger: () => (
<div data-test="mock-info-tooltip-with-trigger" />
),
}));
const defaultProps: ColumnOptionProps = {
column: {

View File

@@ -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();
});

View File

@@ -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', () => () => (
<div data-test="mock-info-tooltip-with-trigger" />
));
jest.mock('../../src/components/InfoTooltipWithTrigger', () => ({
InfoTooltipWithTrigger: () => (
<div data-test="mock-info-tooltip-with-trigger" />
),
}));
jest.mock('../../src/components/ColumnTypeLabel/ColumnTypeLabel', () => ({
ColumnTypeLabel: () => <div data-test="mock-column-type-label" />,
}));

View File

@@ -36,8 +36,8 @@ export default function OptionDescription({ option }: { option: ColumnMeta }) {
<span className="m-r-5 option-label">{option.label}</span>
{option.description && (
<InfoTooltipWithTrigger
className="m-r-5 text-muted"
icon="question-circle-o"
className="m-r-5"
type="question"
tooltip={option.description}
label={`descr-${option.label}`}
/>

View File

@@ -71,7 +71,7 @@ ${helperDescriptions
<div>
{props.label}
<InfoTooltipWithTrigger
iconsStyle={{ marginLeft: theme.sizeUnit }}
iconStyle={{ marginLeft: theme.sizeUnit }}
tooltip={<SafeMarkdown source={helpersTooltipContent} />}
/>
</div>

View File

@@ -48,7 +48,7 @@ const StyleControl = (props: CustomControlConfig<StyleCustomControlProps>) => {
<div>
{props.label}
<InfoTooltipWithTrigger
iconsStyle={{ marginLeft: theme.sizeUnit }}
iconStyle={{ marginLeft: theme.sizeUnit }}
tooltip={t('You need to configure HTML sanitization to use CSS')}
/>
</div>

View File

@@ -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')}
>
<InfoTooltipWithTrigger
icon="line-chart"
placement="top"
label={t('explore')}
/>{' '}
<IconTooltip placement="top" tooltip={t('Explore')} />
<Icons.LineChartOutlined iconSize="m" />
{t('Explore')}
</Button>
);

View File

@@ -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 = ({
<Badge count={paramCount} />
{!isValid && (
<InfoTooltipWithTrigger
icon="exclamation-triangle"
bsStyle="danger"
type="error"
tooltip={t('Invalid JSON')}
label="invalid-json"
/>

View File

@@ -40,19 +40,20 @@ const PLACEMENTS = [
'topRight',
];
const theme = useTheme();
export const InteractiveIconTooltip = (args: IconTooltipProps) => (
<div
css={css`
margin: ${theme.sizeUnit * 10}px ${theme.sizeUnit * 17.5}px;
`}
>
<IconTooltip {...args}>
<Icons.InfoCircleOutlined />
</IconTooltip>
</div>
);
export const InteractiveIconTooltip = (args: IconTooltipProps) => {
const theme = useTheme();
return (
<div
css={css`
margin: ${theme.sizeUnit * 10}px ${theme.sizeUnit * 17.5}px;
`}
>
<IconTooltip {...args}>
<Icons.InfoCircleOutlined />
</IconTooltip>
</div>
);
};
InteractiveIconTooltip.args = {
tooltip: 'Tooltip',

View File

@@ -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';

View File

@@ -63,7 +63,7 @@ export default function PopoverSection({
margin-right: ${theme.sizeUnit}px;
`}
>
<Icons.InfoCircleFilled
<Icons.InfoCircleOutlined
role="img"
iconSize="s"
iconColor={theme.colors.grayscale.light1}

View File

@@ -142,9 +142,10 @@ test('shows and hides the confirmation modal on deactivation', async () => {
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();

View File

@@ -288,7 +288,7 @@ const Row = props => {
<DeleteComponentButton onDelete={handleDeleteComponent} />
<IconButton
onClick={handleChangeFocus}
icon={<Icons.SettingOutlined iconSize="xl" />}
icon={<Icons.SettingOutlined iconSize="l" />}
/>
</HoverMenu>
)}

View File

@@ -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();

View File

@@ -121,7 +121,7 @@ const ControlHeader: FC<ControlHeaderProps> = ({
label={t('bolt')}
tooltip={t('Changing this control takes effect instantly')}
placement="top"
icon="bolt"
type="notice"
/>{' '}
</span>
)}

View File

@@ -192,7 +192,7 @@ class AnnotationLayerControl extends PureComponent<Props, PopoverState> {
return (
<InfoTooltipWithTrigger
label="validation-errors"
bsStyle="danger"
type="error"
tooltip={annotationError[anno.name]}
/>
);

View File

@@ -72,9 +72,8 @@ export default function Option({
<Label data-test="control-label">{children}</Label>
{(!!datasourceWarningMessage || isExtra) && (
<StyledInfoTooltipWithTrigger
icon="exclamation-triangle"
type="warning"
placement="top"
bsStyle="warning"
tooltip={
datasourceWarningMessage ||
t(`

View File

@@ -395,9 +395,8 @@ export const OptionControlLabel = ({
</Label>
{(!!datasourceWarningMessage || isExtra) && (
<StyledInfoTooltipWithTrigger
icon="exclamation-triangle"
type="warning"
placement="top"
bsStyle="warning"
tooltip={
datasourceWarningMessage ||
t(`