diff --git a/superset-frontend/oxlint.json b/superset-frontend/oxlint.json index ee162217cf9..6f168d1f78f 100644 --- a/superset-frontend/oxlint.json +++ b/superset-frontend/oxlint.json @@ -215,6 +215,7 @@ "jsx-a11y/no-noninteractive-tabindex": "error", "jsx-a11y/no-redundant-roles": "error", "jsx-a11y/no-static-element-interactions": "error", + "jsx-a11y/prefer-tag-over-role": "error", "jsx-a11y/role-has-required-aria-props": "error", "jsx-a11y/role-supports-aria-props": "error", "jsx-a11y/scope": "error", diff --git a/superset-frontend/packages/superset-ui-core/src/components/ActionButton/ActionButton.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/ActionButton/ActionButton.test.tsx index 06c6584d592..fb33b4b12cf 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/ActionButton/ActionButton.test.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/ActionButton/ActionButton.test.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { render, screen, userEvent, fireEvent } from '@superset-ui/core/spec'; +import { render, screen, userEvent } from '@superset-ui/core/spec'; import { Icons } from '@superset-ui/core/components/Icons'; import { ActionButton } from '.'; @@ -45,18 +45,6 @@ test('calls onClick when clicked', async () => { expect(onClick).toHaveBeenCalledTimes(1); }); -test('calls onClick when activated with the keyboard', () => { - const onClick = jest.fn(); - render(); - - const button = screen.getByRole('button'); - fireEvent.keyDown(button, { key: 'Enter' }); - expect(onClick).toHaveBeenCalledTimes(1); - - fireEvent.keyDown(button, { key: ' ' }); - expect(onClick).toHaveBeenCalledTimes(2); -}); - test('renders with tooltip when tooltip prop is provided', async () => { const tooltipText = 'This is a tooltip'; render(); @@ -115,6 +103,6 @@ test('has proper accessibility attributes', () => { render(); const button = screen.getByRole('button'); - expect(button).toHaveAttribute('tabIndex', '0'); - expect(button).toHaveAttribute('role', 'button'); + expect(button.tagName).toBe('BUTTON'); + expect(button).toHaveAttribute('type', 'button'); }); diff --git a/superset-frontend/packages/superset-ui-core/src/components/ActionButton/index.tsx b/superset-frontend/packages/superset-ui-core/src/components/ActionButton/index.tsx index 3e56efbc265..e0bb6f560db 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/ActionButton/index.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/ActionButton/index.tsx @@ -17,8 +17,8 @@ * under the License. */ -import { handleKeyboardActivation } from '../../utils'; import type { ReactElement, ReactNode } from 'react'; +import cx from 'classnames'; import { Tooltip, type TooltipPlacement } from '@superset-ui/core/components'; import { css, useTheme } from '@apache-superset/core/theme'; @@ -28,6 +28,9 @@ export interface ActionProps { placement?: TooltipPlacement; icon: ReactNode; onClick: () => void; + className?: string; + disabled?: boolean; + dataTest?: string; } export const ActionButton = ({ @@ -36,30 +39,45 @@ export const ActionButton = ({ placement, icon, onClick, + className, + disabled = false, + dataTest, }: ActionProps) => { const theme = useTheme(); const actionButton = ( - {icon} - + ); const tooltipId = `${label.replaceAll(' ', '-').toLowerCase()}-tooltip`; diff --git a/superset-frontend/packages/superset-ui-core/src/components/ButtonGroup/index.tsx b/superset-frontend/packages/superset-ui-core/src/components/ButtonGroup/index.tsx index 89c373c1e2b..4b1b33cbad8 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/ButtonGroup/index.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/ButtonGroup/index.tsx @@ -21,7 +21,11 @@ import type { ButtonGroupProps } from './types'; export function ButtonGroup(props: ButtonGroupProps) { const { className, children } = props; return ( + // role="group" is the correct ARIA pattern for a generic button toolbar; + // the suggested native tags (fieldset, etc.) carry unrelated form + // semantics and unwanted default browser styling.
since it's a component tree, not an + // image file reference. + // eslint-disable-next-line jsx-a11y/prefer-tag-over-role
css` + appearance: none; + border: none; + background: none; + font: inherit; font-size: ${theme.fontSizeXL}px; display: flex; padding: 0 0 0 ${theme.sizeUnit * 2}px; @@ -55,12 +59,10 @@ export const FaveStar = ({ const content = ( {isStarred ? ( ) } + // input[type=password] doesn't get an implicit "textbox" role + // (unlike other text inputs), so this explicit override is + // needed for it to be discoverable via role-based queries/AT. + // eslint-disable-next-line jsx-a11y/prefer-tag-over-role role="textbox" /> ) : renderAsTextArea ? ( diff --git a/superset-frontend/packages/superset-ui-core/src/components/IconButton/index.tsx b/superset-frontend/packages/superset-ui-core/src/components/IconButton/index.tsx index 31a7899d99b..7c6e35d3d3e 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/IconButton/index.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/IconButton/index.tsx @@ -79,8 +79,12 @@ const IconButton: React.FC = ({ }; return ( + // antd's Card renders a fixed
(no polymorphic tag support) with + // its own rich internal layout (cover/title/tooltip); that doesn't map + // onto a native )} {!isButton && ( + // Generic wrapper used by 19+ callers passing arbitrary + // triggerNode content that relies on block-level
layout; + // swapping to
+ ); }; diff --git a/superset-frontend/packages/superset-ui-core/src/components/PopoverSection/PopoverSection.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/PopoverSection/PopoverSection.test.tsx index 09bbb44be38..734c7311848 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/PopoverSection/PopoverSection.test.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/PopoverSection/PopoverSection.test.tsx @@ -22,7 +22,7 @@ import PopoverSection from '.'; test('renders with default props', async () => { render( -
+
, ); expect(await screen.findByRole('form')).toBeInTheDocument(); @@ -32,7 +32,7 @@ test('renders with default props', async () => { test('renders tooltip icon', async () => { render( -
+ , ); expect((await screen.findAllByRole('img')).length).toBe(2); @@ -41,7 +41,7 @@ test('renders tooltip icon', async () => { test('renders a tooltip when hovered', async () => { render( -
+ , ); await userEvent.hover(screen.getAllByRole('img')[0]); @@ -52,7 +52,7 @@ test('calls onSelect when clicked', async () => { const onSelect = jest.fn(); render( -
+ , ); await userEvent.click(await screen.findByRole('img')); diff --git a/superset-frontend/packages/superset-ui-core/src/components/PopoverSection/index.tsx b/superset-frontend/packages/superset-ui-core/src/components/PopoverSection/index.tsx index 284bc60c824..6f33f72ad2e 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/PopoverSection/index.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/PopoverSection/index.tsx @@ -16,8 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { handleKeyboardActivation } from '../../utils'; -import { ReactNode, SyntheticEvent } from 'react'; +import { MouseEventHandler, ReactNode } from 'react'; import { css, useTheme } from '@apache-superset/core/theme'; import { Icons } from '@superset-ui/core/components/Icons'; import { Tooltip } from '../Tooltip'; @@ -25,10 +24,7 @@ import { Tooltip } from '../Tooltip'; export interface PopoverSectionProps { title: string; isSelected?: boolean; - // `SyntheticEvent` (rather than `MouseEventHandler`) so the same callback - // can be reused as the keyboard-activation handler via - // `handleKeyboardActivation`, which invokes it with a `KeyboardEvent`. - onSelect?: (event: SyntheticEvent) => void; + onSelect?: MouseEventHandler; info?: string; children?: ReactNode; } @@ -48,12 +44,17 @@ export default function PopoverSection({ opacity: isSelected ? 1 : 0.6, }} > -
+ {/* role is auto-computed by BaseIconComponent as "img" since + there's no onClick, so no explicit role needed here. */} @@ -77,10 +79,9 @@ export default function PopoverSection({ )} -
+
({ diff --git a/superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.test.tsx index 527bd235dc1..be5ada50d55 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.test.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.test.tsx @@ -870,14 +870,14 @@ test('Renders only an overflow tag if dropdown is open in oneLine mode', async ( test('does not fire onChange when searching but no selection', async () => { const onChange = jest.fn(); render( -
+
-
, + , ); await open(); await type('Joh'); diff --git a/superset-frontend/packages/superset-ui-core/src/components/Select/Select.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/Select/Select.test.tsx index 9e901f14fce..dbae8d4efe4 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/Select/Select.test.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/Select/Select.test.tsx @@ -1134,14 +1134,14 @@ test('dropdown takes full width of the select input for single select', async () test('does not fire onChange when searching but no selection', async () => { const onChange = jest.fn(); render( -
+
/ +
{Boolean(selectedFlatRows.length) && ( <> - toggleAllRowsSelected(false)} - onKeyDown={handleKeyboardActivation(() => - toggleAllRowsSelected(false), - )} > {t('Deselect all')} - +
{bulkActions .filter( @@ -543,19 +548,15 @@ export function ListView({ ))} {enableBulkTag && ( - setShowBulkTagModal(true)} - onKeyDown={handleKeyboardActivation(() => - setShowBulkTagModal(true), - )} > {t('Add Tag')} - + )} )} diff --git a/superset-frontend/src/components/MessageToasts/Toast.tsx b/superset-frontend/src/components/MessageToasts/Toast.tsx index 7884b615995..656d140919c 100644 --- a/superset-frontend/src/components/MessageToasts/Toast.tsx +++ b/superset-frontend/src/components/MessageToasts/Toast.tsx @@ -152,10 +152,11 @@ export default function Toast({ toast, onCloseToast }: ToastPresenterProps) { {icon}
+ {/* role is auto-computed by BaseIconComponent as "button" since + onClick is present, so no explicit role needed here. */} = ({ return ( (the suggested tag) is for form + // calculation results, not a fit here. + // eslint-disable-next-line jsx-a11y/prefer-tag-over-role role="status" aria-label={`Auto-refresh status: ${displayStatus}`} data-test="status-indicator-dot" diff --git a/superset-frontend/src/dashboard/components/CustomizationsBadge/index.tsx b/superset-frontend/src/dashboard/components/CustomizationsBadge/index.tsx index b13120a3fcf..3406bcc8863 100644 --- a/superset-frontend/src/dashboard/components/CustomizationsBadge/index.tsx +++ b/superset-frontend/src/dashboard/components/CustomizationsBadge/index.tsx @@ -295,6 +295,10 @@ export const CustomizationsBadge = ({ chartId }: CustomizationsBadgeProps) => { diff --git a/superset-frontend/src/dashboard/components/DeleteComponentButton.tsx b/superset-frontend/src/dashboard/components/DeleteComponentButton.tsx index 58c2b91c870..37c24a180f3 100644 --- a/superset-frontend/src/dashboard/components/DeleteComponentButton.tsx +++ b/superset-frontend/src/dashboard/components/DeleteComponentButton.tsx @@ -23,7 +23,7 @@ import type { IconType } from '@superset-ui/core/components/Icons/types'; import IconButton from './IconButton'; type DeleteComponentButtonProps = { - onDelete: MouseEventHandler; + onDelete: MouseEventHandler; iconSize?: IconType['iconSize']; }; diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/DetailsPanel.test.tsx b/superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/DetailsPanel.test.tsx index b20ac55f74e..19dde396cfa 100644 --- a/superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/DetailsPanel.test.tsx +++ b/superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/DetailsPanel.test.tsx @@ -37,7 +37,7 @@ const mockPopoverTriggerRef = { current: { focus: jest.fn(), }, -} as unknown as RefObject; +} as unknown as RefObject; const createProps = () => ({ popoverVisible: true, diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/index.tsx b/superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/index.tsx index 80f70e3189c..e9c2bae0613 100644 --- a/superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/index.tsx +++ b/superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/index.tsx @@ -39,7 +39,7 @@ export interface DetailsPanelProps { children: JSX.Element; popoverVisible: boolean; popoverContentRef: RefObject; - popoverTriggerRef: RefObject; + popoverTriggerRef: RefObject; setPopoverVisible: (visible: boolean) => void; } diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx b/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx index c74c9edd2a4..58d47e6bc01 100644 --- a/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx +++ b/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx @@ -55,8 +55,11 @@ export interface FiltersBadgeProps { chartId: number; } -const StyledFilterCount = styled.div` +const StyledFilterCount = styled.button` ${({ theme }) => ` + appearance: none; + border: none; + font: inherit; display: flex; justify-items: center; align-items: center; @@ -135,7 +138,7 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => { ); const [popoverVisible, setPopoverVisible] = useState(false); const popoverContentRef = useRef(null); - const popoverTriggerRef = useRef(null); + const popoverTriggerRef = useRef(null); const onHighlightFilterSource = useCallback( (path: string[]) => { @@ -144,7 +147,7 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => { [dispatch], ); - const handleKeyDown = (event: KeyboardEvent) => { + const handleKeyDown = (event: KeyboardEvent) => { if (event.key === 'Enter') { setPopoverVisible(true); } @@ -306,15 +309,14 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => { popoverTriggerRef={popoverTriggerRef} > diff --git a/superset-frontend/src/dashboard/components/IconButton.tsx b/superset-frontend/src/dashboard/components/IconButton.tsx index d497f603a6b..3e48730cff1 100644 --- a/superset-frontend/src/dashboard/components/IconButton.tsx +++ b/superset-frontend/src/dashboard/components/IconButton.tsx @@ -19,10 +19,10 @@ import { forwardRef, HTMLAttributes, MouseEventHandler } from 'react'; import { styled, SupersetTheme } from '@apache-superset/core/theme'; -interface IconButtonProps extends HTMLAttributes { +interface IconButtonProps extends HTMLAttributes { icon: JSX.Element; label?: string; - onClick: MouseEventHandler; + onClick: MouseEventHandler; disabled?: boolean; 'data-test'?: string; } @@ -39,7 +39,11 @@ const activeCss = ({ theme }: { theme: SupersetTheme }) => ` } `; -const StyledDiv = styled.div<{ isDisabled?: boolean }>` +const StyledButton = styled.button<{ isDisabled?: boolean }>` + appearance: none; + border: none; + background: none; + font: inherit; display: flex; align-items: center; cursor: pointer; @@ -54,7 +58,7 @@ const StyledSpan = styled.span` margin-left: ${({ theme }) => theme.sizeUnit * 2}px; `; -const IconButton = forwardRef( +const IconButton = forwardRef( ( { icon, @@ -67,11 +71,10 @@ const IconButton = forwardRef( }, ref, ) => ( - ( > {icon} {label && {label}} - + ), ); diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx b/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx index 6727efb263e..122e29ffce3 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx @@ -170,7 +170,7 @@ const createOwnStateWithChartState = ( const Chart = (props: ChartProps) => { const dispatch = useDispatch(); - const descriptionRef = useRef(null); + const descriptionRef = useRef(null); const headerRef = useRef(null); const boundActionCreators = useMemo( @@ -756,14 +756,13 @@ const Chart = (props: ChartProps) => { https://github.com/apache/superset/pull/23862 */} {isExpanded && slice.description_markdown && ( -
)} diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.tsx b/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.tsx index 6f6621c7282..04d6bb537e4 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.tsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.tsx @@ -16,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ -import { handleKeyboardActivation } from '@superset-ui/core'; import { Fragment, useCallback, @@ -28,7 +27,7 @@ import { } from 'react'; import classNames from 'classnames'; import { useDispatch, useSelector } from 'react-redux'; -import { styled } from '@apache-superset/core/theme'; +import { css, styled } from '@apache-superset/core/theme'; import { t } from '@apache-superset/core/translation'; import { EditableTitle, EmptyState } from '@superset-ui/core/components'; @@ -348,16 +347,21 @@ const Tab = (props: TabProps): ReactElement => { ) : ( {t('You can add the components in the')}{' '} - dispatch(setEditMode(true))} - onKeyDown={handleKeyboardActivation(() => - dispatch(setEditMode(true)), - )} + css={css` + appearance: none; + border: none; + background: none; + padding: 0; + font: inherit; + cursor: pointer; + text-decoration: underline; + `} > {t('edit mode')} - + )) } diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tabs/Tabs.tsx b/superset-frontend/src/dashboard/components/gridComponents/Tabs/Tabs.tsx index 2175342c8b2..d1f351be0db 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tabs/Tabs.tsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tabs/Tabs.tsx @@ -98,8 +98,6 @@ interface ShowDropIndicatorsResult { interface CloseIconWithDropIndicatorProps { showDropIndicators: ShowDropIndicatorsResult; - role?: string; - tabIndex?: number; } const CloseIconWithDropIndicator = ( @@ -481,9 +479,9 @@ const Tabs = (props: TabsProps): ReactElement => { ), closeIcon: ( + // rc-tabs already wraps closeIcon content in its own + // setCurrentChartId(undefined)} className={activeChartId === undefined ? 'active' : ''} > diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/VerticalCollapse.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/VerticalCollapse.tsx index 7389d70ede8..e57da65a55d 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/VerticalCollapse.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/VerticalCollapse.tsx @@ -47,6 +47,12 @@ const CrossFiltersVerticalCollapse = (props: { const sectionHeaderStyle = useCallback( (theme: SupersetTheme) => css` + appearance: none; + border: none; + background: none; + font: inherit; + text-align: left; + width: 100%; display: flex; align-items: center; justify-content: space-between; @@ -119,21 +125,10 @@ const CrossFiltersVerticalCollapse = (props: { return (
{!hideHeader && ( -
{ - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault(); - toggleSection(); - } - }} - role="button" - tabIndex={0} - > +
+ )} {isOpen &&
{crossFiltersIndicators}
} {isOpen &&
} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink/index.tsx index c68cf9953e4..6f2790c913c 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink/index.tsx @@ -16,8 +16,8 @@ * specific language governing permissions and limitations * under the License. */ -import { handleKeyboardActivation } from '@superset-ui/core'; import { ReactNode, FC, memo } from 'react'; +import { css } from '@apache-superset/core/theme'; import { getFilterBarTestId } from '../utils'; @@ -30,15 +30,20 @@ export const FilterConfigurationLink: FC = ({ onClick, children, }) => ( -
{children} -
+ ); export default memo(FilterConfigurationLink); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControlShared.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControlShared.tsx index 0d03c96cf0f..33263d76ca3 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControlShared.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControlShared.tsx @@ -236,6 +236,10 @@ export const DescriptionToolTip = ({ whiteSpace: 'normal', }} > + {/* Deliberate role="button" on this tooltip-trigger icon (no click + handler) — mirrors DeckglLayerVisibilityTooltip below; not a fit + for the linter's suggested