Skip to content

Commit c4cbbd6

Browse files
V-halfaroV-halfaro
authored
Add house IconButton and migrate the secondary bar off Primer React (#63742)
1 parent 9aac668 commit c4cbbd6

11 files changed

Lines changed: 283 additions & 126 deletions

File tree

‎src/fixtures/tests/playwright-rendering.spec.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -830,6 +830,23 @@ test.describe('test nav at different viewports', () => {
830830
await expect(page.getByTestId('sidebar')).toBeHidden()
831831
})
832832

833+
test('secondary-bar icon buttons show a label tooltip that Escape dismisses', async ({
834+
page,
835+
}) => {
836+
await page.setViewportSize({ width: 1400, height: 700 })
837+
await page.goto('/get-started/foo/bar')
838+
839+
const toggle = page.getByRole('button', { name: 'Collapse sidebar' })
840+
await expect(toggle).not.toHaveAttribute('aria-label')
841+
const tooltip = page.locator(`[id="${await toggle.getAttribute('aria-labelledby')}"]`)
842+
await expect(tooltip).toBeHidden()
843+
await toggle.focus()
844+
await expect(tooltip).toBeVisible()
845+
await expect(tooltip).toHaveText('Collapse sidebar')
846+
await page.keyboard.press('Escape')
847+
await expect(tooltip).toBeHidden()
848+
})
849+
833850
for (const { name, width } of [
834851
{ name: 'medium viewports - 768-1011', width: 1000 },
835852
{ name: 'small viewports - 544-767', width: 555 },

‎src/frame/components/page-header/BreadcrumbsScroller.module.scss‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,12 @@
3535
background-color: var(--brand-color-canvas-default);
3636
}
3737

38-
// Keep the solid canvas on hover because Primer's invisible IconButton uses a
39-
// translucent tint that lets crumbs bleed through. A currentColor stroke thickens
40-
// the fill-based octicon glyph for hover feedback.
38+
// Keep the solid canvas on hover because the invisible variant's translucent tint
39+
// lets crumbs bleed through. A currentColor stroke thickens the fill-based octicon
40+
// glyph for hover feedback.
4141
.leftChevron:hover,
4242
.rightChevron:hover {
43-
background-color: var(--brand-color-canvas-default) !important;
43+
background-color: var(--brand-color-canvas-default);
4444
color: var(--brand-color-text-default, #000000);
4545

4646
svg {

‎src/frame/components/page-header/BreadcrumbsScroller.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { useCallback, useEffect, useRef, useState } from 'react'
22
import type { FocusEvent } from 'react'
33
import cx from 'clsx'
4-
import { IconButton } from '@primer/react'
54
import { ChevronLeftIcon, ChevronRightIcon } from '@primer/octicons-react'
65

6+
import { IconButton } from '@/frame/components/ui/IconButton'
77
import { useTranslation } from '@/languages/components/useTranslation'
88
import { Breadcrumbs } from './Breadcrumbs'
99

‎src/frame/components/page-header/DocsSecondaryBar.module.scss‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,8 @@
6565
}
6666

6767
// Override primer/css color-fg-muted, whose grey clashes with brand-painted breadcrumbs.
68-
// Keep !important because PRC's invisible IconButton variant uses .prc-Button-* rules
69-
// that otherwise win.
7068
.toggleIcon {
71-
color: var(--brand-color-text-muted) !important;
69+
color: var(--brand-color-text-muted);
7270
}
7371

7472
// The In this article row sits directly beneath the breadcrumb bar. DefaultLayout

‎src/frame/components/page-header/DocsSecondaryBar.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import cx from 'clsx'
22
import { useRouter } from 'next/router'
3-
import { IconButton } from '@primer/react'
43
import { SidebarCollapseIcon, SidebarExpandIcon } from '@primer/octicons-react'
54

5+
import { IconButton } from '@/frame/components/ui/IconButton'
66
import { useMainContext } from '@/frame/components/context/MainContext'
77
import { useTranslation } from '@/languages/components/useTranslation'
88
import { useSidebarCollapsed } from '@/frame/components/sidebar/SidebarCollapseContext'

‎src/frame/components/page-header/Header.tsx‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,7 @@ export const Header = ({ isNarrowMenuOpen, onNarrowMenuToggle }: Props) => {
9494
onClick={handleClick}
9595
leadingComponent={<VersionPicker variant="header" onNavigate={closeNarrowMenu} />}
9696
trailingComponent={
97-
languagePickerVisible ? (
98-
<LanguagePicker variant="header" onNavigate={closeNarrowMenu} />
99-
) : undefined
97+
languagePickerVisible ? <LanguagePicker onNavigate={closeNarrowMenu} /> : undefined
10098
}
10199
>
102100
<SubdomainNavBar.Search

‎src/frame/components/ui/IconButton/IconButton.module.scss‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Variant and size rules sit in :where() so consumer classes override them
2+
// without !important.
3+
.iconButton {
4+
display: inline-flex;
5+
align-items: center;
6+
justify-content: center;
7+
flex: none;
8+
padding: 0;
9+
// Colour lives in the variants. Invisible's transparent border keeps an outline in forced-colors mode.
10+
border-width: 1px;
11+
border-style: solid;
12+
border-radius: var(--brand-borderRadius-medium);
13+
cursor: pointer;
14+
15+
&:focus-visible {
16+
outline: 2px solid var(--brand-color-focus);
17+
outline-offset: 2px;
18+
}
19+
}
20+
21+
:where(.iconButton[data-size="small"]) {
22+
width: 28px;
23+
height: 28px;
24+
}
25+
26+
:where(.iconButton[data-size="medium"]) {
27+
width: 32px;
28+
height: 32px;
29+
}
30+
31+
:where(.iconButton[data-variant="default"]) {
32+
color: var(--brand-color-text-default);
33+
background-color: var(--brand-button-secondary-bgColor-rest);
34+
border-color: var(--brand-button-secondary-borderColor-rest);
35+
36+
&:hover {
37+
background-color: var(--brand-button-secondary-bgColor-hover);
38+
border-color: var(--brand-button-secondary-borderColor-hover);
39+
}
40+
41+
&:active {
42+
background-color: var(--brand-button-secondary-bgColor-active);
43+
border-color: var(--brand-button-secondary-borderColor-active);
44+
}
45+
}
46+
47+
:where(.iconButton[data-variant="invisible"]) {
48+
color: var(--brand-color-text-muted);
49+
background-color: transparent;
50+
border-color: transparent;
51+
52+
&:hover {
53+
background-color: var(--brand-button-subtle-bgColor-hover);
54+
}
55+
56+
&:active {
57+
background-color: var(--brand-button-subtle-bgColor-active);
58+
}
59+
}

‎src/frame/components/ui/IconButton/IconButton.tsx‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import type { ButtonHTMLAttributes, ElementType } from 'react'
2+
import cx from 'clsx'
3+
import { Tooltip } from '@primer/react-brand'
4+
5+
import styles from '@/frame/components/ui/IconButton/IconButton.module.scss'
6+
7+
export type IconButtonProps = Omit<
8+
ButtonHTMLAttributes<HTMLButtonElement>,
9+
'aria-label' | 'children' | 'type'
10+
> & {
11+
icon: ElementType
12+
'aria-label': string
13+
variant?: 'default' | 'invisible'
14+
size?: 'small' | 'medium'
15+
tooltip?: boolean
16+
tooltipDirection?: 'n' | 'e' | 's' | 'w'
17+
}
18+
19+
// With a tooltip, the tooltip text labels the button through aria-labelledby, so
20+
// the button carries no aria-label and screen readers hear one name. Hidden buttons
21+
// skip the tooltip because nothing can hover or focus them.
22+
export const IconButton = ({
23+
icon: Icon,
24+
'aria-label': ariaLabel,
25+
variant = 'default',
26+
size = 'medium',
27+
tooltip = true,
28+
tooltipDirection = 's',
29+
className,
30+
...props
31+
}: IconButtonProps) => {
32+
const isHidden = props['aria-hidden'] === true || props['aria-hidden'] === 'true'
33+
const withTooltip = tooltip && !isHidden
34+
35+
const button = (
36+
<button
37+
type="button"
38+
className={cx(styles.iconButton, className)}
39+
data-variant={variant}
40+
data-size={size}
41+
aria-label={withTooltip ? undefined : ariaLabel}
42+
{...props}
43+
>
44+
<Icon size={16} />
45+
</button>
46+
)
47+
48+
if (!withTooltip) return button
49+
50+
return (
51+
<Tooltip type="label" text={ariaLabel} direction={tooltipDirection}>
52+
{button}
53+
</Tooltip>
54+
)
55+
}

‎src/frame/components/ui/IconButton/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export { IconButton } from '@/frame/components/ui/IconButton/IconButton'
2+
export type { IconButtonProps } from '@/frame/components/ui/IconButton/IconButton'

‎src/frame/components/ui/IconButton/tests/icon-button.ts‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import { createElement } from 'react'
2+
import { describe, expect, test } from 'vitest'
3+
import { renderToStaticMarkup } from 'react-dom/server'
4+
import { load } from 'cheerio'
5+
import { SidebarExpandIcon } from '@primer/octicons-react'
6+
7+
import { IconButton } from '@/frame/components/ui/IconButton'
8+
import type { IconButtonProps } from '@/frame/components/ui/IconButton'
9+
10+
const LABEL = 'Collapse sidebar'
11+
12+
function render(props: Partial<IconButtonProps> = {}) {
13+
const html = renderToStaticMarkup(
14+
createElement(IconButton, { icon: SidebarExpandIcon, 'aria-label': LABEL, ...props }),
15+
)
16+
return load(html)
17+
}
18+
19+
describe('IconButton', () => {
20+
test('labels the button through its tooltip, without a second aria-label', () => {
21+
const $ = render()
22+
const button = $('button')
23+
expect(button.attr('aria-label')).toBeUndefined()
24+
const labelId = button.attr('aria-labelledby')
25+
expect(labelId).toBeTruthy()
26+
const tooltip = $(`[id="${labelId}"]`)
27+
expect(tooltip.text()).toBe(LABEL)
28+
expect(tooltip.attr('aria-hidden')).toBe('true')
29+
})
30+
31+
test('uses aria-label and renders no tooltip when the tooltip is off', () => {
32+
const $ = render({ tooltip: false })
33+
expect($('button').attr('aria-label')).toBe(LABEL)
34+
expect($('button').attr('aria-labelledby')).toBeUndefined()
35+
expect($('div').length).toBe(0)
36+
})
37+
38+
test('renders no tooltip for an aria-hidden button', () => {
39+
const $ = render({ 'aria-hidden': true, tabIndex: -1 })
40+
expect($('button').attr('aria-label')).toBe(LABEL)
41+
expect($('button').attr('aria-hidden')).toBe('true')
42+
expect($('div').length).toBe(0)
43+
})
44+
45+
test('is a type=button and passes through button attributes', () => {
46+
const $ = render({
47+
'data-testid': 'sidebar-collapse-toggle',
48+
'aria-expanded': true,
49+
tabIndex: 0,
50+
className: 'consumer',
51+
} as Partial<IconButtonProps>)
52+
const button = $('button')
53+
expect(button.attr('type')).toBe('button')
54+
expect(button.attr('data-testid')).toBe('sidebar-collapse-toggle')
55+
expect(button.attr('aria-expanded')).toBe('true')
56+
expect(button.attr('tabindex')).toBe('0')
57+
expect(button.hasClass('consumer')).toBe(true)
58+
})
59+
60+
test('defaults to the default variant at medium size', () => {
61+
const $ = render()
62+
expect($('button').attr('data-variant')).toBe('default')
63+
expect($('button').attr('data-size')).toBe('medium')
64+
expect(render({ variant: 'invisible', size: 'small' })('button').attr('data-size')).toBe(
65+
'small',
66+
)
67+
})
68+
})

0 commit comments

Comments
 (0)