From 7710793f7470b8b0028b18aab11c9520f9855c0d Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Tue, 28 Jun 2022 17:39:26 -0400 Subject: [PATCH 01/22] renders a tooltip for icon-only segmented control buttons --- .../SegmentedControlIconButton.tsx | 35 ++++++++---- .../getSegmentedControlStyles.ts | 54 +++++++++++-------- 2 files changed, 59 insertions(+), 30 deletions(-) diff --git a/src/SegmentedControl/SegmentedControlIconButton.tsx b/src/SegmentedControl/SegmentedControlIconButton.tsx index 3f6f9f0f075..e865dc86706 100644 --- a/src/SegmentedControl/SegmentedControlIconButton.tsx +++ b/src/SegmentedControl/SegmentedControlIconButton.tsx @@ -2,7 +2,11 @@ import React, {HTMLAttributes} from 'react' import {IconProps} from '@primer/octicons-react' import styled from 'styled-components' import sx, {merge, SxProp} from '../sx' -import getSegmentedControlButtonStyles from './getSegmentedControlStyles' +import getSegmentedControlButtonStyles, { + borderedSegment, + directChildLayoutAdjustments +} from './getSegmentedControlStyles' +import Tooltip from '../Tooltip' export type SegmentedControlIconButtonProps = { 'aria-label': string @@ -17,10 +21,11 @@ const SegmentedControlIconButtonStyled = styled.button` ${sx}; ` -// TODO: get tooltips working: -// - by default, the tooltip shows the `ariaLabel` content -// - allow users to pass custom tooltip text +// TODO: update this component to be accessible when we update the Tooltip component +// - we wouldn't render tooltip content inside a pseudoelement +// - users can pass custom tooltip text in addition to `ariaLabel` export const SegmentedControlIconButton: React.FC = ({ + 'aria-label': ariaLabel, icon: Icon, selected, sx: sxProp = {}, @@ -29,11 +34,23 @@ export const SegmentedControlIconButton: React.FC - - - - + + + + + + + ) } diff --git a/src/SegmentedControl/getSegmentedControlStyles.ts b/src/SegmentedControl/getSegmentedControlStyles.ts index c462b2c9910..34c93ac2a18 100644 --- a/src/SegmentedControl/getSegmentedControlStyles.ts +++ b/src/SegmentedControl/getSegmentedControlStyles.ts @@ -1,6 +1,29 @@ import {get} from '../constants' import {SegmentedControlButtonProps} from './SegmentedControlButton' +export const directChildLayoutAdjustments = { + ':first-child': { + marginLeft: '-1px' + }, + + ':last-child': { + marginRight: '-1px' + } +} + +export const borderedSegment = { + marginRight: '1px', + ':after': { + backgroundColor: 'var(--separator-color)', + content: '""', + position: 'absolute', + right: '-2px', + top: 2, + bottom: 2, + width: '1px' + } +} + const getSegmentedControlButtonStyles = (props?: SegmentedControlButtonProps & {isIconOnly?: boolean}) => ({ '--segmented-control-button-inner-padding': '12px', // TODO: use primitive `primer.control.medium.paddingInline.normal` when it is available '--segmented-control-button-bg-inset': '4px', @@ -18,7 +41,10 @@ const getSegmentedControlButtonStyles = (props?: SegmentedControlButtonProps & { marginBottom: '-1px', padding: props?.selected ? 0 : 'var(--segmented-control-button-bg-inset)', position: 'relative', - width: props?.isIconOnly ? 'var(--primer-control-medium-size, 32px)' : undefined, + ...(props?.isIconOnly && { + height: 'var(--primer-control-medium-size, 32px)', + width: 'var(--primer-control-medium-size, 32px)' + }), '.segmentedControl-content': { alignItems: 'center', @@ -54,26 +80,12 @@ const getSegmentedControlButtonStyles = (props?: SegmentedControlButtonProps & { backgroundColor: props?.selected ? undefined : 'actionListItem.default.activeBg' // TODO: replace with a functional primitive }, - ':first-child': { - marginLeft: '-1px' - }, - - ':last-child': { - marginRight: '-1px' - }, - - ':not(:last-child)': { - marginRight: '1px', - ':after': { - backgroundColor: 'var(--separator-color)', - content: '""', - position: 'absolute', - right: '-2px', - top: 2, - bottom: 2, - width: '1px' - } - }, + // Icon-only buttons render the button inside of an element rendered by the Tooltip component. + // This breaks `:first-child` and `:last-child` selectors on buttons because the button is the only child inside of the element rendered by the Tooltip component. + ...(!props?.isIconOnly && { + ...directChildLayoutAdjustments, + ':not(:last-child)': borderedSegment + }), '.segmentedControl-text': { ':after': { From 9927ea58b8ebb013350c7105315ab8c496e91a8f Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 29 Jun 2022 17:35:04 -0400 Subject: [PATCH 02/22] implements responsive variant prop --- src/SegmentedControl/SegmentedControl.tsx | 153 +++++++++++++++--- .../SegmentedControlButton.tsx | 7 +- .../SegmentedControlIconButton.tsx | 5 +- src/SegmentedControl/examples.stories.tsx | 91 +++++++++-- .../getSegmentedControlStyles.ts | 6 +- src/hooks/useMatchMedia.ts | 63 ++++++++ src/utils/types/ViewportRangeKeys.ts | 1 + 7 files changed, 286 insertions(+), 40 deletions(-) create mode 100644 src/hooks/useMatchMedia.ts create mode 100644 src/utils/types/ViewportRangeKeys.ts diff --git a/src/SegmentedControl/SegmentedControl.tsx b/src/SegmentedControl/SegmentedControl.tsx index efb9767e0a2..5038705d7c4 100644 --- a/src/SegmentedControl/SegmentedControl.tsx +++ b/src/SegmentedControl/SegmentedControl.tsx @@ -1,8 +1,10 @@ import React from 'react' import Button, {SegmentedControlButtonProps} from './SegmentedControlButton' import SegmentedControlIconButton, {SegmentedControlIconButtonProps} from './SegmentedControlIconButton' -import {Box, useTheme} from '..' +import {ActionList, ActionMenu, Box, useTheme} from '..' import {merge, SxProp} from '../sx' +import useMatchMedia from '../hooks/useMatchMedia' +import {ViewportRangeKeys} from '../utils/types/ViewportRangeKeys' type SegmentedControlProps = { 'aria-label'?: string @@ -12,6 +14,7 @@ type SegmentedControlProps = { fullWidth?: boolean /** The handler that gets called when a segment is selected */ onChange?: (selectedIndex: number) => void // TODO: consider making onChange required if we force this component to be controlled + variant?: Partial> } & SxProp const getSegmentedControlStyles = (props?: SegmentedControlProps) => ({ @@ -27,18 +30,42 @@ const getSegmentedControlStyles = (props?: SegmentedControlProps) => ({ height: '32px' // TODO: use primitive `primer.control.medium.size` when it is available }) -// TODO: implement `variant` prop for responsive behavior -// TODO: implement `loading` prop // TODO: log a warning if no `ariaLabel` or `ariaLabelledBy` prop is passed // TODO: implement keyboard behavior to move focus using the arrow keys -const Root: React.FC = ({children, fullWidth, onChange, sx: sxProp = {}, ...rest}) => { +const Root: React.FC = ({children, fullWidth, onChange, sx: sxProp = {}, variant, ...rest}) => { const {theme} = useTheme() - const selectedChildren = React.Children.toArray(children).map( + const matchesMedia = useMatchMedia(Object.keys(variant || {}) as ViewportRangeKeys[]) + const matchesMediaKeys = matchesMedia ? (Object.keys(matchesMedia) as ViewportRangeKeys[]) : [] + const selectedSegments = React.Children.toArray(children).map( child => React.isValidElement(child) && child.props.selected ) - const hasSelectedButton = selectedChildren.some(isSelected => isSelected) - const selectedIndex = hasSelectedButton ? selectedChildren.indexOf(true) : 0 + const hasSelectedButton = selectedSegments.some(isSelected => isSelected) + const selectedIndex = hasSelectedButton ? selectedSegments.indexOf(true) : 0 + const selectedChild = React.isValidElement( + React.Children.toArray(children)[selectedIndex] + ) + ? React.Children.toArray(children)[selectedIndex] + : undefined + const getChildIcon = (childArg: React.ReactNode) => { + if ( + React.isValidElement(childArg) && + childArg.type === Button && + childArg.props.leadingIcon + ) { + return childArg.props.leadingIcon + } + + return React.isValidElement(childArg) ? childArg.props.icon : null + } + const getChildText = (childArg: React.ReactNode) => { + if (React.isValidElement(childArg) && childArg.type === Button) { + return childArg.props.children + } + + return React.isValidElement(childArg) ? childArg.props['aria-label'] : null + } + const sx = merge( getSegmentedControlStyles({ fullWidth @@ -46,24 +73,106 @@ const Root: React.FC = ({children, fullWidth, onChange, s sxProp as SxProp ) - return ( + const shouldHideLabels = matchesMediaKeys.some(size => { + if (matchesMedia && variant && typeof matchesMedia === 'object') { + return matchesMedia[size] && variant[size] === 'hideLabels' + } + }) + + const shouldRenderDropdown = matchesMediaKeys.some(size => { + if (matchesMedia && variant && typeof matchesMedia === 'object') { + return matchesMedia[size] && variant[size] === 'dropdown' + } + }) + + return shouldRenderDropdown ? ( + // Render the 'dropdown' variant of the SegmentedControlButton or SegmentedControlIconButton + + {getChildText(selectedChild)} + + + {React.Children.map(children, (child, index) => { + const ChildIcon = getChildIcon(child) + // Not a valid child element - skip rendering + if (!React.isValidElement(child)) { + return null + } + + return ( + | React.KeyboardEvent) => { + onChange(index) + // TODO: figure out a way around the typecasting + child.props.onClick && child.props.onClick(event as React.MouseEvent) + } + : // TODO: figure out a way around the typecasting + (child.props.onClick as ( + event: React.MouseEvent | React.KeyboardEvent + ) => void) + } + > + {ChildIcon && } {getChildText(child)} + + ) + })} + + + + ) : ( - {React.Children.map(children, (child, i) => { - if (React.isValidElement(child)) { - return React.cloneElement(child, { - onClick: onChange - ? (e: React.MouseEvent) => { - onChange(i) - child.props.onClick && child.props.onClick(e) + {React.Children.map(children, (child, index) => { + // Not a valid child element - render nothing + if (!React.isValidElement(child)) { + return null + } + const sharedChildProps = { + onClick: onChange + ? (event: React.MouseEvent) => { + onChange(index) + child.props.onClick && child.props.onClick(event) + } + : child.props.onClick, + selected: index === selectedIndex, + sx: { + '--separator-color': + index === selectedIndex || index === selectedIndex - 1 ? 'transparent' : theme?.colors.border.default + } as React.CSSProperties + } + + // Render the 'hideLabels' variant of the SegmentedControlButton + if (React.isValidElement(child) && child.type === Button && shouldHideLabels) { + const {'aria-label': ariaLabel, leadingIcon, children: childPropsChildren, ...restChildProps} = child.props + if (!childPropsChildren && !ariaLabel) { + // eslint-disable-next-line no-console + console.warn('A `children` or `aria-label` prop is required when hiding visible labels') + } else if (!leadingIcon) { + // eslint-disable-next-line no-console + console.warn('A `leadingIcon` prop is required when hiding visible labels') + } else { + return ( + + ) + } } + + // Default - render the children as-is, but add the shared child props + return React.cloneElement(child, sharedChildProps) })} ) diff --git a/src/SegmentedControl/SegmentedControlButton.tsx b/src/SegmentedControl/SegmentedControlButton.tsx index 7aacd5cb4d4..727fed82c03 100644 --- a/src/SegmentedControl/SegmentedControlButton.tsx +++ b/src/SegmentedControl/SegmentedControlButton.tsx @@ -3,16 +3,17 @@ import {IconProps} from '@primer/octicons-react' import styled from 'styled-components' import {Box} from '..' import sx, {merge, SxProp} from '../sx' -import getSegmentedControlButtonStyles from './getSegmentedControlStyles' +import {getSegmentedControlButtonStyles} from './getSegmentedControlStyles' export type SegmentedControlButtonProps = { - children?: string + /** The visible label rendered in the button */ + children: string /** Whether the segment is selected */ selected?: boolean /** The leading icon comes before item label */ leadingIcon?: React.FunctionComponent } & SxProp & - HTMLAttributes + HTMLAttributes const SegmentedControlButtonStyled = styled.button` ${sx}; diff --git a/src/SegmentedControl/SegmentedControlIconButton.tsx b/src/SegmentedControl/SegmentedControlIconButton.tsx index e865dc86706..da7a88cc7cb 100644 --- a/src/SegmentedControl/SegmentedControlIconButton.tsx +++ b/src/SegmentedControl/SegmentedControlIconButton.tsx @@ -2,8 +2,9 @@ import React, {HTMLAttributes} from 'react' import {IconProps} from '@primer/octicons-react' import styled from 'styled-components' import sx, {merge, SxProp} from '../sx' -import getSegmentedControlButtonStyles, { +import { borderedSegment, + getSegmentedControlButtonStyles, directChildLayoutAdjustments } from './getSegmentedControlStyles' import Tooltip from '../Tooltip' @@ -15,7 +16,7 @@ export type SegmentedControlIconButtonProps = { /** Whether the segment is selected */ selected?: boolean } & SxProp & - HTMLAttributes + HTMLAttributes const SegmentedControlIconButtonStyled = styled.button` ${sx}; diff --git a/src/SegmentedControl/examples.stories.tsx b/src/SegmentedControl/examples.stories.tsx index cef34a873c3..3a0edc96113 100644 --- a/src/SegmentedControl/examples.stories.tsx +++ b/src/SegmentedControl/examples.stories.tsx @@ -2,13 +2,42 @@ import React, {useState} from 'react' import {Meta} from '@storybook/react' import {BaseStyles, ThemeProvider} from '..' -import {ComponentProps} from '../utils/types' import {SegmentedControl} from '.' import {EyeIcon, FileCodeIcon, PeopleIcon} from '@primer/octicons-react' -type Args = ComponentProps +type ResponsiveVariantOptions = 'dropdown' | 'hideLabels' +type Args = { + fullWidth?: boolean + variantAtNarrow: ResponsiveVariantOptions + variantAtNarrowLandscape: ResponsiveVariantOptions + variantAtRegular: ResponsiveVariantOptions + variantAtWide: ResponsiveVariantOptions + variantAtPortrait: ResponsiveVariantOptions + variantAtLandscape: ResponsiveVariantOptions +} + +const excludedControlKeys = ['aria-label', 'onChange', 'sx', 'variant'] + +const variantOptions = ['dropdown', 'hideLabels', undefined] -const excludedControlKeys = ['aria-label', 'onChange', 'sx'] +const parseVarientFromArgs = (args: Args) => { + const { + variantAtNarrow, + variantAtNarrowLandscape, + variantAtRegular, + variantAtWide, + variantAtPortrait, + variantAtLandscape + } = args + return { + narrow: variantAtNarrow, + narrowLandscape: variantAtNarrowLandscape, + regular: variantAtRegular, + wide: variantAtWide, + portrait: variantAtPortrait, + atLandscape: variantAtLandscape + } +} export default { title: 'SegmentedControl/examples', @@ -20,10 +49,52 @@ export default { type: 'boolean' } }, - loading: { - defaultValue: false, + variantAtNarrow: { + name: 'variant.narrow', + defaultValue: undefined, control: { - type: 'boolean' + type: 'radio', + options: variantOptions + } + }, + variantAtNarrowLandscape: { + name: 'variant.narrowLandscape', + defaultValue: undefined, + control: { + type: 'radio', + options: variantOptions + } + }, + variantAtRegular: { + name: 'variant.regular', + defaultValue: undefined, + control: { + type: 'radio', + options: variantOptions + } + }, + variantAtWide: { + name: 'variant.wide', + defaultValue: undefined, + control: { + type: 'radio', + options: variantOptions + } + }, + variantAtPortrait: { + name: 'variant.portrait', + defaultValue: undefined, + control: { + type: 'radio', + options: variantOptions + } + }, + variantAtLandscape: { + name: 'variant.Landscape', + defaultValue: undefined, + control: { + type: 'radio', + options: variantOptions } } }, @@ -42,7 +113,7 @@ export default { } as Meta export const Default = (args: Args) => ( - + Preview Raw Blame @@ -56,7 +127,7 @@ export const Controlled = (args: Args) => { } return ( - + Preview Raw Blame @@ -65,7 +136,7 @@ export const Controlled = (args: Args) => { } export const WithIconsAndLabels = (args: Args) => ( - + Preview @@ -75,7 +146,7 @@ export const WithIconsAndLabels = (args: Args) => ( ) export const IconsOnly = (args: Args) => ( - + diff --git a/src/SegmentedControl/getSegmentedControlStyles.ts b/src/SegmentedControl/getSegmentedControlStyles.ts index 34c93ac2a18..0aa1947b9f5 100644 --- a/src/SegmentedControl/getSegmentedControlStyles.ts +++ b/src/SegmentedControl/getSegmentedControlStyles.ts @@ -24,7 +24,9 @@ export const borderedSegment = { } } -const getSegmentedControlButtonStyles = (props?: SegmentedControlButtonProps & {isIconOnly?: boolean}) => ({ +export const getSegmentedControlButtonStyles = ( + props?: Partial> & {isIconOnly?: boolean} +) => ({ '--segmented-control-button-inner-padding': '12px', // TODO: use primitive `primer.control.medium.paddingInline.normal` when it is available '--segmented-control-button-bg-inset': '4px', '--segmented-control-outer-radius': get('radii.2')(props), @@ -112,5 +114,3 @@ const getSegmentedControlButtonStyles = (props?: SegmentedControlButtonProps & { } } }) - -export default getSegmentedControlButtonStyles diff --git a/src/hooks/useMatchMedia.ts b/src/hooks/useMatchMedia.ts new file mode 100644 index 00000000000..dc78ed7ffb8 --- /dev/null +++ b/src/hooks/useMatchMedia.ts @@ -0,0 +1,63 @@ +import {useState, useLayoutEffect} from 'react' +import {ViewportRangeKeys} from '../utils/types/ViewportRangeKeys' + +// TODO: import from Primer Primitives https://primer.style/primitives/spacing/ +const primer = { + viewportRange: { + narrow: '(max-width: calc(48rem - 0.02px))', + narrowLandscape: '(max-width: calc(63.25rem - 0.02px))', + regular: '(min-width: 48rem)', + wide: '(min-width: 90rem)', + portrait: '(orientation: portrait)', + landscape: '(orientation: portrait)' + } +} + +const useMatchMedia = (viewportRange?: ViewportRangeKeys | ViewportRangeKeys[]) => { + const [matchesByKey, setMatchesByKey] = useState>({ + narrow: undefined, + narrowLandscape: undefined, + regular: undefined, + wide: undefined, + portrait: undefined, + landscape: undefined + }) + + useLayoutEffect(() => { + const updateMatches = (viewportRangeKeyArg: ViewportRangeKeys) => { + const media = window.matchMedia(primer.viewportRange[viewportRangeKeyArg]) + + if (media.matches !== matchesByKey[viewportRangeKeyArg]) { + setMatchesByKey({...matchesByKey, [viewportRangeKeyArg]: media.matches}) + } + + const listener = () => { + setMatchesByKey({...matchesByKey, [viewportRangeKeyArg]: media.matches}) + } + + media.addEventListener('change', listener) + + // TODO: figure out how remove event listeners in a for...of loop + return () => { + media.removeEventListener('change', listener) + } + } + + if (!viewportRange || !viewportRange.length) { + return + } + + if (typeof viewportRange === 'string') { + return updateMatches(viewportRange) + } + + // can't remove event listeners in a for..of loop because I'd have to do `return updateMatches()`, but then the loop stops on return + for (const rangeKey of viewportRange) { + updateMatches(rangeKey) + } + }, [matchesByKey, viewportRange]) + + return typeof viewportRange === 'string' ? matchesByKey[viewportRange] : matchesByKey +} + +export default useMatchMedia diff --git a/src/utils/types/ViewportRangeKeys.ts b/src/utils/types/ViewportRangeKeys.ts new file mode 100644 index 00000000000..74f0cf379dd --- /dev/null +++ b/src/utils/types/ViewportRangeKeys.ts @@ -0,0 +1 @@ +export type ViewportRangeKeys = 'narrow' | 'narrowLandscape' | 'regular' | 'wide' | 'portrait' | 'landscape' From 566af62f07f12597c4c7abc3e9d29c62e0f4bd8f Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Thu, 30 Jun 2022 13:12:44 -0400 Subject: [PATCH 03/22] adds tests --- .../SegmentedControl.test.tsx | 138 +++++++++++++++++- src/SegmentedControl/SegmentedControl.tsx | 5 +- 2 files changed, 137 insertions(+), 6 deletions(-) diff --git a/src/SegmentedControl/SegmentedControl.test.tsx b/src/SegmentedControl/SegmentedControl.test.tsx index 30f574e984f..713f808238c 100644 --- a/src/SegmentedControl/SegmentedControl.test.tsx +++ b/src/SegmentedControl/SegmentedControl.test.tsx @@ -1,10 +1,12 @@ import React from 'react' import '@testing-library/jest-dom/extend-expect' -import {render} from '@testing-library/react' +import {render, fireEvent, waitFor} from '@testing-library/react' import {EyeIcon, FileCodeIcon, PeopleIcon} from '@primer/octicons-react' import userEvent from '@testing-library/user-event' import {behavesAsComponent, checkExports, checkStoriesForAxeViolations} from '../utils/testing' import {SegmentedControl} from '.' // TODO: update import when we move this to the global index +import theme from '../theme' +import {BaseStyles, SSRProvider, ThemeProvider} from '..' const segmentData = [ {label: 'Preview', iconLabel: 'EyeIcon', icon: () => }, @@ -14,6 +16,30 @@ const segmentData = [ // TODO: improve test coverage describe('SegmentedControl', () => { + const mockWarningFn = jest.fn() + + beforeAll(() => { + jest.spyOn(global.console, 'warn').mockImplementation(mockWarningFn) + }) + + afterAll(() => { + jest.clearAllMocks() + }) + + Object.defineProperty(window, 'matchMedia', { + writable: true, + value: jest.fn().mockImplementation(query => ({ + matches: true, + media: query, + onchange: null, + addListener: jest.fn(), // deprecated + removeListener: jest.fn(), // deprecated + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + dispatchEvent: jest.fn() + })) + }) + behavesAsComponent({ Component: SegmentedControl, toRender: () => ( @@ -46,6 +72,39 @@ describe('SegmentedControl', () => { expect(selectedButton?.getAttribute('aria-current')).toBe('true') }) + it('renders the dropdown variant', () => { + const {getByText} = render( + + {segmentData.map(({label}, index) => ( + + {label} + + ))} + + ) + const button = getByText(segmentData[1].label) + + expect(button).toBeInTheDocument() + expect(button.closest('button')?.getAttribute('aria-haspopup')).toBe('true') + }) + + it('renders the hideLabels variant', () => { + const {getByLabelText} = render( + + {segmentData.map(({label, icon}, index) => ( + + {label} + + ))} + + ) + + for (const datum of segmentData) { + const labelledButton = getByLabelText(datum.label) + expect(labelledButton).toBeDefined() + } + }) + it('renders the first segment as selected if no child has the `selected` prop passed', () => { const {getByText} = render( @@ -133,7 +192,82 @@ describe('SegmentedControl', () => { } expect(handleClick).toHaveBeenCalled() }) + + it('calls onChange with index of clicked segment button when using the dropdown variant', async () => { + const handleChange = jest.fn() + const component = render( + + + + + {segmentData.map(({label}, index) => ( + + {label} + + ))} + + + + + ) + const button = component.getByText(segmentData[0].label) + + fireEvent.click(button) + expect(handleChange).not.toHaveBeenCalled() + const menuItems = await waitFor(() => component.getAllByRole('menuitemradio')) + fireEvent.click(menuItems[1]) + + expect(handleChange).toHaveBeenCalledWith(1) + }) + + it('calls segment button onClick if it is passed when using the dropdown variant', async () => { + const handleClick = jest.fn() + const component = render( + + + + + {segmentData.map(({label}, index) => ( + + {label} + + ))} + + + + + ) + const button = component.getByText(segmentData[0].label) + + fireEvent.click(button) + expect(handleClick).not.toHaveBeenCalled() + const menuItems = await waitFor(() => component.getAllByRole('menuitemradio')) + fireEvent.click(menuItems[1]) + + expect(handleClick).toHaveBeenCalled() + }) + + it('warns users if they try to use the hideLabels variant without a leadingIcon', () => { + const consoleSpy = jest.spyOn(global.console, 'warn') + + render( + + {segmentData.map(({label}, index) => ( + + {label} + + ))} + + ) + + expect(consoleSpy).toHaveBeenCalled() + }) }) -checkStoriesForAxeViolations('examples', '../SegmentedControl/') +// TODO: uncomment these tests after we fix a11y for the Tooltip component +// checkStoriesForAxeViolations('examples', '../SegmentedControl/') checkStoriesForAxeViolations('fixtures', '../SegmentedControl/') diff --git a/src/SegmentedControl/SegmentedControl.tsx b/src/SegmentedControl/SegmentedControl.tsx index 5038705d7c4..d5b325f3ec7 100644 --- a/src/SegmentedControl/SegmentedControl.tsx +++ b/src/SegmentedControl/SegmentedControl.tsx @@ -146,10 +146,7 @@ const Root: React.FC = ({children, fullWidth, onChange, s // Render the 'hideLabels' variant of the SegmentedControlButton if (React.isValidElement(child) && child.type === Button && shouldHideLabels) { const {'aria-label': ariaLabel, leadingIcon, children: childPropsChildren, ...restChildProps} = child.props - if (!childPropsChildren && !ariaLabel) { - // eslint-disable-next-line no-console - console.warn('A `children` or `aria-label` prop is required when hiding visible labels') - } else if (!leadingIcon) { + if (!leadingIcon) { // eslint-disable-next-line no-console console.warn('A `leadingIcon` prop is required when hiding visible labels') } else { From 4c394defbf20da23662d7be712a4c24c80758384 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Thu, 30 Jun 2022 14:26:28 -0400 Subject: [PATCH 04/22] minor story tweaks --- src/SegmentedControl/examples.stories.tsx | 33 ++++++++++++++--------- src/SegmentedControl/fixtures.stories.tsx | 3 +-- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/SegmentedControl/examples.stories.tsx b/src/SegmentedControl/examples.stories.tsx index 3a0edc96113..fa3bda5f9f8 100644 --- a/src/SegmentedControl/examples.stories.tsx +++ b/src/SegmentedControl/examples.stories.tsx @@ -4,6 +4,7 @@ import {Meta} from '@storybook/react' import {BaseStyles, ThemeProvider} from '..' import {SegmentedControl} from '.' import {EyeIcon, FileCodeIcon, PeopleIcon} from '@primer/octicons-react' +import Box from '../Box' type ResponsiveVariantOptions = 'dropdown' | 'hideLabels' type Args = { @@ -136,19 +137,27 @@ export const Controlled = (args: Args) => { } export const WithIconsAndLabels = (args: Args) => ( - - - Preview - - Raw - Blame - + // padding needed to show Tooltip + // there is a separate initiative to change Tooltip to get positioned with useAnchoredPosition + + + + Preview + + Raw + Blame + + ) export const IconsOnly = (args: Args) => ( - - - - - + // padding needed to show Tooltip + // there is a separate initiative to change Tooltip to get positioned with useAnchoredPosition + + + + + + + ) diff --git a/src/SegmentedControl/fixtures.stories.tsx b/src/SegmentedControl/fixtures.stories.tsx index 888a8d36b20..5dce12899c9 100644 --- a/src/SegmentedControl/fixtures.stories.tsx +++ b/src/SegmentedControl/fixtures.stories.tsx @@ -20,8 +20,7 @@ export default { ] } as Meta -// TODO: make it possible to use FormControl -// - FormControl.Label needs to accept `id` prop +// TODO: make it possible to use FormControl as a wrapper for SegmentedControl // - FormControl.Label needs to accept a prop that lets it render an element that isn't a `