From c5b46a3eef4cfda44e25b42a5f90eadc9ee21c74 Mon Sep 17 00:00:00 2001 From: Bartosz Leper Date: Fri, 6 Dec 2024 18:16:09 +0100 Subject: [PATCH] [v17] Partial rewrite of the slide tabs component (#49458) * Partial rewrite of the slide tabs component This enables us to put icons there, make the layout tighter and fit to tab list contents. It also improves accessibility by adding keyboard handling and changing the structure according to ARIA Authoring Practices Guide. * Make the ID attribute mandatory * review * wip --- .../design/src/SlideTabs/SlideTabs.story.tsx | 121 +++++-- .../design/src/SlideTabs/SlideTabs.test.tsx | 99 ++++-- .../design/src/SlideTabs/SlideTabs.tsx | 300 ++++++++++++++---- 3 files changed, 403 insertions(+), 117 deletions(-) diff --git a/web/packages/design/src/SlideTabs/SlideTabs.story.tsx b/web/packages/design/src/SlideTabs/SlideTabs.story.tsx index 46ff166c5cef7..fa2ce7dbafa2b 100644 --- a/web/packages/design/src/SlideTabs/SlideTabs.story.tsx +++ b/web/packages/design/src/SlideTabs/SlideTabs.story.tsx @@ -18,17 +18,40 @@ import React, { useState } from 'react'; +import * as Icon from 'design/Icon'; +import Flex from 'design/Flex'; + import { SlideTabs } from './SlideTabs'; export default { title: 'Design/SlideTabs', }; +const threeSimpleTabs = [ + { key: 'aws', title: 'aws' }, + { key: 'automatically', title: 'automatically' }, + { key: 'manually', title: 'manually' }, +]; + +const fiveSimpleTabs = [ + { key: 'step1', title: 'step1' }, + { key: 'step2', title: 'step2' }, + { key: 'step3', title: 'step3' }, + { key: 'step4', title: 'step4' }, + { key: 'step5', title: 'step5' }, +]; + +const titlesWithIcons = [ + { key: 'alarm', icon: Icon.AlarmRing, title: 'Clocks' }, + { key: 'bots', icon: Icon.Bots, title: 'Bots' }, + { key: 'check', icon: Icon.Check, title: 'Checkmarks' }, +]; + export const ThreeTabs = () => { const [activeIndex, setActiveIndex] = useState(0); return ( @@ -39,7 +62,7 @@ export const FiveTabs = () => { const [activeIndex, setActiveIndex] = useState(0); return ( @@ -47,34 +70,94 @@ export const FiveTabs = () => { }; export const Round = () => { - const [activeIndex, setActiveIndex] = useState(0); + const [activeIndex1, setActiveIndex1] = useState(0); + const [activeIndex2, setActiveIndex2] = useState(0); return ( - + + + + ); }; export const Medium = () => { - const [activeIndex, setActiveIndex] = useState(0); + const [activeIndex1, setActiveIndex1] = useState(0); + const [activeIndex2, setActiveIndex2] = useState(0); return ( - + + + + + ); +}; + +export const Small = () => { + const [activeIndex1, setActiveIndex1] = useState(0); + const [activeIndex2, setActiveIndex2] = useState(0); + return ( + + + + + ); }; export const LoadingTab = () => { return ( null} activeIndex={1} isProcessing={true} @@ -85,7 +168,7 @@ export const LoadingTab = () => { export const DisabledTab = () => { return ( null} activeIndex={1} disabled={true} diff --git a/web/packages/design/src/SlideTabs/SlideTabs.test.tsx b/web/packages/design/src/SlideTabs/SlideTabs.test.tsx index 12034f7b5f8aa..4636655d25700 100644 --- a/web/packages/design/src/SlideTabs/SlideTabs.test.tsx +++ b/web/packages/design/src/SlideTabs/SlideTabs.test.tsx @@ -21,13 +21,17 @@ import { screen } from '@testing-library/react'; import { render, userEvent } from 'design/utils/testing'; -import { SlideTabs } from './SlideTabs'; +import { SlideTabs, SlideTabsProps } from './SlideTabs'; describe('design/SlideTabs', () => { it('renders the supplied number of tabs(3)', () => { render( {}} activeIndex={0} /> @@ -35,15 +39,21 @@ describe('design/SlideTabs', () => { expect(screen.getAllByRole('tab')).toHaveLength(3); - expect(screen.getByLabelText('aws')).toBeInTheDocument(); - expect(screen.getByLabelText('automatically')).toBeInTheDocument(); - expect(screen.getByLabelText('manually')).toBeInTheDocument(); + expect(getTab('aws')).toBeInTheDocument(); + expect(getTab('automatically')).toBeInTheDocument(); + expect(getTab('manually')).toBeInTheDocument(); }); it('renders the supplied number of tabs(5)', () => { render( {}} activeIndex={0} /> @@ -51,44 +61,70 @@ describe('design/SlideTabs', () => { expect(screen.getAllByRole('tab')).toHaveLength(5); - expect(screen.getByLabelText('aws')).toBeInTheDocument(); - expect(screen.getByLabelText('automatically')).toBeInTheDocument(); - expect(screen.getByLabelText('manually')).toBeInTheDocument(); - expect(screen.getByLabelText('apple')).toBeInTheDocument(); - expect(screen.getByLabelText('purple')).toBeInTheDocument(); - }); - - it('respects a custom form name', () => { - const { container } = render( - {}} - activeIndex={0} - /> - ); - - // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access - expect(container.querySelectorAll('input[name=pineapple]')).toHaveLength(3); + expect(getTab('aws')).toBeInTheDocument(); + expect(getTab('automatically')).toBeInTheDocument(); + expect(getTab('manually')).toBeInTheDocument(); + expect(getTab('apple')).toBeInTheDocument(); + expect(getTab('purple')).toBeInTheDocument(); }); test('onChange highlights the tab clicked', async () => { render(); // First tab is selected by default. - expect(screen.getByRole('tab', { name: 'first' })).toHaveClass('selected'); + expect(getTab('first')).toHaveClass('selected'); // Select the second tab. await userEvent.click(screen.getByText('second')); - expect(screen.getByRole('tab', { name: 'second' })).toHaveClass('selected'); + expect(getTab('second')).toHaveClass('selected'); + + expect(getTab('first')).not.toHaveClass('selected'); + }); - expect(screen.getByRole('tab', { name: 'first' })).not.toHaveClass( - 'selected' + test('keyboard navigation and accessibility', async () => { + const user = userEvent.setup(); + render( + ); + expect(getTab('first')).not.toHaveFocus(); + expect(getTab('second')).not.toHaveFocus(); + + getTab('first').focus(); + expect(getTab('first')).toHaveAttribute('aria-selected', 'true'); + expect(getTab('first')).toHaveAttribute('aria-controls', 'tabpanel-1'); + expect(getTab('second')).toHaveAttribute('aria-selected', 'false'); + expect(getTab('second')).toHaveAttribute('aria-controls', 'tabpanel-2'); + + await user.keyboard('{Right}'); + expect(getTab('first')).toHaveAttribute('aria-selected', 'false'); + expect(getTab('second')).toHaveAttribute('aria-selected', 'true'); + expect(getTab('second')).toHaveFocus(); + + // Should be a no-op. + await user.keyboard('{Right}'); + expect(getTab('first')).toHaveAttribute('aria-selected', 'false'); + expect(getTab('second')).toHaveAttribute('aria-selected', 'true'); + expect(getTab('second')).toHaveFocus(); + + await user.keyboard('{Left}'); + expect(getTab('first')).toHaveAttribute('aria-selected', 'true'); + expect(getTab('second')).toHaveAttribute('aria-selected', 'false'); + expect(getTab('first')).toHaveFocus(); + + // Should be a no-op. + await user.keyboard('{Left}'); + expect(getTab('first')).toHaveAttribute('aria-selected', 'true'); + expect(getTab('second')).toHaveAttribute('aria-selected', 'false'); + expect(getTab('first')).toHaveFocus(); }); }); -const Component = () => { +const Component = (props: Partial) => { const [activeIndex, setActiveIndex] = useState(0); return ( @@ -96,6 +132,9 @@ const Component = () => { onChange={setActiveIndex} tabs={['first', 'second']} activeIndex={activeIndex} + {...props} /> ); }; + +const getTab = (name: string) => screen.getByRole('tab', { name }); diff --git a/web/packages/design/src/SlideTabs/SlideTabs.tsx b/web/packages/design/src/SlideTabs/SlideTabs.tsx index 59f636fb8f0f7..6b92c5803fdf0 100644 --- a/web/packages/design/src/SlideTabs/SlideTabs.tsx +++ b/web/packages/design/src/SlideTabs/SlideTabs.tsx @@ -16,65 +16,122 @@ * along with this program. If not, see . */ -import React from 'react'; +import React, { useEffect, useRef } from 'react'; import styled from 'styled-components'; -import { Indicator, Text } from '..'; +import { Flex, Indicator } from 'design'; +import { IconProps } from 'design/Icon/Icon'; export function SlideTabs({ appearance = 'square', activeIndex = 0, - name = 'slide-tab', onChange, - size = 'xlarge', + size = 'large', tabs, isProcessing = false, disabled = false, -}: props) { + fitContent = false, +}: SlideTabsProps) { + const activeTab = useRef(null); + const tabContainer = useRef(null); + + useEffect(() => { + // Note: this is important for accessibility; the screen reader may ignore + // tab changing if we focus the tab list, and not the tab. + if (tabContainer.current?.contains?.(document.activeElement)) { + activeTab.current?.focus(); + } + }, [activeIndex]); + + function handleKeyDown(e: React.KeyboardEvent) { + if (e.key === 'ArrowRight' && activeIndex < tabs.length - 1) { + onChange(activeIndex + 1); + } + if (e.key === 'ArrowLeft' && activeIndex > 0) { + onChange(activeIndex - 1); + } + } + + // The component structure was designed according to + // https://www.w3.org/WAI/ARIA/apg/patterns/tabs/examples/tabs-automatic/. return ( - - - {tabs.map((tabName, tabIndex) => { + // A container that displays background and sets up padding for the slider + // area. It's separate from the tab list itself, since we need to + // absolutely position the slider relative to this container's content box, + // and not its padding box. So we set up padding if needed on this one, and + // then position the slider against the tab list. + + + {tabs.map((tabSpec, tabIndex) => { const selected = tabIndex === activeIndex; - let onClick; + const { + key, + title, + icon: Icon, + ariaLabel, + controls, + } = toFullTabSpec(tabSpec, tabIndex); + + let onClick = undefined; if (!disabled && !isProcessing) { - onClick = (e: React.MouseEvent) => { + onClick = (e: React.MouseEvent) => { e.preventDefault(); onChange(tabIndex); }; } + return ( - - + {/* We need a separate tab content component, since the spinner, + when displayed, shouldn't take up space to prevent layout + jumping. TabContent serves as a positioning anchor whose left + edge is the left edge of the content (not the tab button, + which can be much wider). */} + {selected && isProcessing && } - {tabName} - - - + {Icon && } + {title} + + ); })} - - + {/* The tab slider is positioned absolutely and appears below the + actual tab button. The outer component is responsible for + establishing the part of parent's width where the slider appears, + and the internal slider may (or may not, depending on the control + size) include additional padding that separates tabs. */} + + + + ); } -type props = { +export type SlideTabsProps = { /** * The style to render the selector in. */ @@ -83,11 +140,6 @@ type props = { * The index that you'd like to select on the initial render. */ activeIndex: number; - /** - * The name you'd like to use for the form if using multiple tabs on the page. - * Default: "slide-tab" - */ - name?: string; /** * To be notified when the selected tab changes supply it with this fn. */ @@ -95,11 +147,11 @@ type props = { /** * The size to render the selector in. */ - size?: 'xlarge' | 'medium'; + size?: Size; /** - * A list of tab names that you'd like displayed in the list of tabs. + * A list of tab specs that you'd like displayed in the list of tabs. */ - tabs: string[]; + tabs: TabSpec[]; /** * If true, renders a spinner and disables clicking on the tabs. * @@ -112,77 +164,189 @@ type props = { * If true, disables pointer events. */ disabled?: boolean; + /** + * If true, the control doesn't take as much horizontal space as possible, + * but instead wraps its contents. + */ + fitContent?: boolean; }; -export type TabComponent = { - name: string; - component: React.ReactNode; +/** + * Definition of a tab. If it's a string, it denotes a title displayed on the + * tab. It's recommended to use a full object with tab panel ID for better + * accessibility. + * + * TODO(bl-nero): remove the string option once Enterprise is migrated to + * simplify it a bit. + */ +type TabSpec = string | FullTabSpec; + +type FullTabSpec = TabContentSpec & { + /** Iteration key for the tab. */ + key: React.Key; + /** + * ID of the controlled element for accessibility, perhaps autogenerated + * with an `useId()` hook. The indicated element should have a `role` + * attribute set to "tabpanel". + */ + controls?: string; }; -const Wrapper = styled.div` +/** + * Tab content. Either an icon with a mandatory accessible label, or a + * decorative icon with accompanying text. + */ +type TabContentSpec = + | { + /** Title displayed on the tab. */ + title: string; + ariaLabel?: never; + /** Icon displayed on the tab. */ + icon?: React.ComponentType; + } + | { + title?: never; + /** Accessible label for the tab. */ + ariaLabel: string; + /** Icon displayed on the tab. */ + icon: React.ComponentType; + }; + +function toFullTabSpec(spec: TabSpec, index: number): FullTabSpec { + if (typeof spec !== 'string') return spec; + return { + key: index, + title: spec, + }; +} + +const TabSliderInner = styled.div<{ appearance: Appearance }>` + height: 100%; + background-color: ${({ theme }) => theme.colors.brand}; + border-radius: ${props => (props.appearance === 'square' ? '8px' : '60px')}; +`; + +const Wrapper = styled.div<{ + fitContent: boolean; + size: Size; + appearance: Appearance; +}>` position: relative; + ${props => (props.fitContent ? 'width: fit-content;' : '')} + /* + * For the small size, we don't use paddings between tab buttons. Therefore, + * the area of tab list is evenly divided into segments, and we anchor the + * slider relative to the box with horizontal padding. With larger sizes, we + * expect to have some distance between the tab buttons. It means that the + * positions of the slider, expressed as relative to what the padding box + * would be, are no longer proportional to the tab index (there is distance + * between the tabs, but no distance on the left of the first tab and on the + * right of the last tab). Therefore, to calculate the position of slider as + * a percentage of its container's width, we set the wrapper's horizontal + * padding to 0, thus giving us a couple of pixels of breathing room; now we + * can go back to using a linear formula to calculate the slider position. + * This lack of padding will be then compensated for by adjusting tab button + * margins appropriately. + */ + padding: ${props => (props.size === 'small' ? '4px 4px' : '8px 0')}; + background-color: ${props => props.theme.colors.interactive.tonal.neutral[0]}; + border-radius: ${props => (props.appearance === 'square' ? '8px' : '60px')}; + + &:has(:focus-visible) ${TabSliderInner} { + outline: 2px solid ${props => props.theme.colors.brand}; + outline-offset: 1px; + } `; -const TabLabel = styled.label<{ - itemCount: number; +const tabButtonHeight = ({ size }: { size: Size }) => { + switch (size) { + case 'large': + return { height: '40px' }; + case 'medium': + return { height: '36px' }; + case 'small': + return { height: '32px' }; + } +}; + +const TabButton = styled.button<{ processing?: boolean; disabled?: boolean; + selected?: boolean; + size: Size; }>` + /* Reset the button styles. */ + font-family: inherit; + text-decoration: inherit; + outline: none; + border: none; + background: transparent; + padding: ${props => (props.size === 'small' ? '8px 8px' : '8px 16px')}; + + ${props => props.theme.typography.body2} + cursor: ${p => (p.processing || p.disabled ? 'default' : 'pointer')}; display: flex; + align-items: center; justify-content: center; - padding: 10px; - width: ${props => 100 / props.itemCount}%; z-index: 1; /* Ensures that the label is above the background slider. */ opacity: ${p => (p.processing || p.disabled ? 0.5 : 1)}; -`; + ${tabButtonHeight} + /* + * Using similar logic as with wrapper padding, we compensate for the lack of + * thereof with button margins if needed. + */ + margin: 0 ${props => (props.size === 'small' ? '0' : '8px')}; + color: ${props => + props.selected + ? props.theme.colors.text.primaryInverse + : props.theme.colors.text.main}; -const TabInput = styled.input` - display: none; + transition: color 0.2s ease-in 0s; `; type Appearance = 'square' | 'round'; -type Size = 'xlarge' | 'medium'; +type Size = 'large' | 'medium' | 'small'; const TabSlider = styled.div<{ - appearance: Appearance; itemCount: number; size: Size; activeIndex: number; }>` - background-color: ${({ theme }) => theme.colors.brand}; - border-radius: ${props => (props.appearance === 'square' ? '8px' : '60px')}; - box-shadow: 0px 2px 6px rgba(12, 12, 14, 0.1); - height: ${props => (props.size === 'xlarge' ? '56px' : '40px')}; - left: calc(${props => (100 / props.itemCount) * props.activeIndex}% + 8px); - margin: ${props => - props.size === 'xlarge' ? '12px 12px 12px 0' : '4px 4px 4px 0'}; + box-sizing: border-box; position: absolute; + left: ${props => (100 / props.itemCount) * props.activeIndex}%; top: 0; - transition: all 0.3s ease; - width: calc(${props => 100 / props.itemCount}% - 16px); + bottom: 0; + width: ${props => 100 / props.itemCount}%; + padding: 0 ${props => (props.size === 'small' ? '0' : '8px')}; + transition: + all 0.3s ease, + outline 0s, + outline-offset 0s; `; -const TabNav = styled.nav<{ appearance: Appearance; size: Size }>` +const TabList = styled.div<{ itemCount: number }>` + position: relative; align-items: center; - background-color: ${props => props.theme.colors.spotBackground[0]}; - border-radius: ${props => (props.appearance === 'square' ? '8px' : '60px')}; - display: flex; - height: ${props => (props.size === 'xlarge' ? '80px' : '47px')}; + /* + * Grid display allows us to allocate equal amount of space for every tab + * (which is important for calculating the slider position) and at the same + * time support the "fit content" mode. (It's impossible to do in the flex + * layout.) + */ + display: grid; + grid-template-columns: repeat(${props => props.itemCount}, 1fr); justify-content: space-around; color: ${props => props.theme.colors.text.main}; - .selected { - color: ${props => props.theme.colors.text.primaryInverse}; - transition: color 0.2s ease-in 0s; - } `; const Spinner = styled(Indicator)` color: ${p => p.theme.colors.levels.deep}; position: absolute; - left: -${p => p.theme.space[4]}px; + left: -${p => p.theme.space[5]}px; `; -const Box = styled.div` +const TabContent = styled(Flex)` position: relative; `;