diff --git a/.changeset/curly-gorillas-happen.md b/.changeset/curly-gorillas-happen.md new file mode 100644 index 000000000..83e180255 --- /dev/null +++ b/.changeset/curly-gorillas-happen.md @@ -0,0 +1,9 @@ +--- +'@primer/react-brand': patch +--- + +`SubNav` accessibility improvements: + +- Removed focus-trapping in the menu overlay on narrow viewports +- Added hover state to the `SubNav.Heading` +- Added `aria-current` visual indicator parity on narrow viewports diff --git a/.github/workflows/visual_test_storybook.yml b/.github/workflows/visual_test_storybook.yml index 11dfa3804..f437aa747 100644 --- a/.github/workflows/visual_test_storybook.yml +++ b/.github/workflows/visual_test_storybook.yml @@ -7,7 +7,7 @@ jobs: ci: if: ${{ github.repository == 'primer/brand' }} name: Storybook - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 env: NODE_ENV: test steps: @@ -17,7 +17,7 @@ jobs: - name: Set up Node uses: actions/setup-node@v2 with: - node-version: 18 + node-version: 20 - name: Cache dependencies uses: actions/cache@v4 @@ -43,19 +43,13 @@ jobs: run: NODE_ENV=test npx start-server-and-test 'npx http-server ./apps/storybook/storybook-static -p 6006' 6006 'cd packages/e2e && npx playwright test' continue-on-error: true - - name: Upload error screenshots - uses: actions/upload-artifact@v3 - if: steps.playwright-step.outcome != 'success' - with: - name: playwright-test-results - path: playwright-test-results - - name: Upload test results - if: always() - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 + if: ${{ !cancelled() }} with: name: playwright-report - path: playwright-report + path: playwright-report/ + retention-days: 30 - name: Comment on the PR about no visual differences if: steps.playwright-step.outcome != 'success' diff --git a/package-lock.json b/package-lock.json index 552d4a163..f30225acc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -46,7 +46,7 @@ }, "apps/storybook": { "name": "@primer/brand-storybook", - "version": "0.43.0", + "version": "0.44.0", "license": "MIT", "devDependencies": { "@babel/preset-env": "^7.22.0", @@ -28590,7 +28590,7 @@ }, "packages/design-tokens": { "name": "@primer/brand-primitives", - "version": "0.43.0", + "version": "0.44.0", "license": "MIT", "devDependencies": { "@primer/primitives": "9.1.1", @@ -28603,7 +28603,7 @@ }, "packages/e2e": { "name": "@primer/brand-e2e", - "version": "0.43.0", + "version": "0.44.0", "license": "MIT", "devDependencies": { "@github/axe-github": "^0.5.0", @@ -28617,7 +28617,7 @@ }, "packages/fonts": { "name": "@primer/brand-fonts", - "version": "0.43.0", + "version": "0.44.0", "license": "MIT", "engines": { "node": ">=16.0.0", @@ -28626,7 +28626,7 @@ }, "packages/react": { "name": "@primer/react-brand", - "version": "0.43.0", + "version": "0.44.0", "license": "MIT", "dependencies": { "@oddbird/popover-polyfill": "0.4.3", @@ -29862,7 +29862,7 @@ }, "packages/repo-configs": { "name": "@primer/brand-config", - "version": "0.43.0", + "version": "0.44.0", "license": "MIT" } }, diff --git a/packages/e2e/scripts/playwright/run-visual-tests b/packages/e2e/scripts/playwright/run-visual-tests index 8ac435811..ecd6ed1e6 100755 --- a/packages/e2e/scripts/playwright/run-visual-tests +++ b/packages/e2e/scripts/playwright/run-visual-tests @@ -1,2 +1,2 @@ cd ../../apps/storybook && npx storybook build && cd ../../packages/e2e -docker run --rm --ipc=host -v ${PWD%/*/*}:/tests mcr.microsoft.com/playwright:v1.24.0-focal /bin/bash -c "cd tests && npm install && npx playwright install chrome && npx playwright install && npx start-server-and-test 'npx http-server ./apps/storybook/storybook-static -p 6006' 6006 'cd packages/e2e && npx playwright test'" +docker run --rm --ipc=host -v ${PWD%/*/*}:/tests mcr.microsoft.com/playwright:v1.24.0-focal /bin/bash -c "cd tests && npm install && npx playwright install chromium && npx playwright install && npx start-server-and-test 'npx http-server ./apps/storybook/storybook-static -p 6006' 6006 'cd packages/e2e && npx playwright test'" diff --git a/packages/e2e/scripts/playwright/update-visual-snapshots b/packages/e2e/scripts/playwright/update-visual-snapshots index 921aa5da0..4d79e9dc4 100755 --- a/packages/e2e/scripts/playwright/update-visual-snapshots +++ b/packages/e2e/scripts/playwright/update-visual-snapshots @@ -1,2 +1,2 @@ cd ../../apps/storybook && npx storybook build && cd ../../packages/e2e -docker run --rm --ipc=host -v ${PWD%/*/*}:/tests mcr.microsoft.com/playwright:v1.37.0-jammy /bin/bash -c "cd tests && npm install && npx playwright install && npx start-server-and-test 'npx http-server ./apps/storybook/storybook-static -p 6006' 6006 'cd packages/e2e && npx playwright test --update-snapshots'" \ No newline at end of file +docker run --rm --ipc=host -v ${PWD%/*/*}:/tests mcr.microsoft.com/playwright:v1.46.0-noble /bin/bash -c "cd tests && npm install && npx playwright install && npx start-server-and-test 'npx http-server ./apps/storybook/storybook-static -p 6006' 6006 'cd packages/e2e && npx playwright test --update-snapshots'" \ No newline at end of file diff --git a/packages/react/src/SubNav/SubNav.module.css b/packages/react/src/SubNav/SubNav.module.css index 5c94d88c8..fd70cea5a 100644 --- a/packages/react/src/SubNav/SubNav.module.css +++ b/packages/react/src/SubNav/SubNav.module.css @@ -24,7 +24,7 @@ } .SubNav__heading:hover { - color: var(--brand-SubNav-color-link-active); + color: var(--brand-color-text-muted); text-decoration: none !important; /* dotcom override */ } @@ -333,6 +333,10 @@ text-decoration: none; } + .SubNav__links-overlay--open .SubNav__link-label { + position: relative; + } + .SubNav__links-overlay--open .SubNav__link[aria-current]:not([aria-current='false']) * { color: var(--brand-SubNav-color-link-active); } @@ -394,6 +398,7 @@ .SubNav__link-label { font-size: var(--brand-text-size-200); font-weight: var(--base-text-weight-semibold); + padding-block-end: var(--base-size-6); } .SubNav__sub-menu--anchor { @@ -418,9 +423,7 @@ position: relative; } - .SubNav .SubNav__link:hover .SubNav__link-label::after, - .SubNav .SubNav__link:focus-visible .SubNav__link-label::after, - .SubNav .SubNav__link[aria-current]:not([aria-current='false']) .SubNav__link-label::after { + .SubNav .SubNav__link--has-sub-menu:hover .SubNav__sub-menu-children .SubNav__link-label::after { display: none; } } diff --git a/packages/react/src/SubNav/SubNav.tsx b/packages/react/src/SubNav/SubNav.tsx index 4dae8220f..e255be12d 100644 --- a/packages/react/src/SubNav/SubNav.tsx +++ b/packages/react/src/SubNav/SubNav.tsx @@ -21,7 +21,6 @@ import {default as clsx} from 'clsx' import {ChevronDownIcon, ChevronUpIcon} from '@primer/octicons-react' import {useId} from '@reach/auto-id' import {useKeyboardEscape} from '../hooks/useKeyboardEscape' -import {useFocusTrap} from '../hooks/useFocusTrap' import {useOnClickOutside} from '../hooks/useOnClickOutside' import {useProvidedRefOrCreate} from '../hooks/useRef' import {useContainsFocus} from './useContainsFocus' @@ -154,7 +153,6 @@ const _SubNavRoot = memo(({id, children, className, 'data-testid': testId, fullW useOnClickOutside(rootRef, closeMenuCallback) useKeyboardEscape(closeMenuCallback) - useFocusTrap({containerRef: overlayRef, restoreFocusOnCleanUp: true, disabled: !isOpenAtNarrow}) useEffect(() => { if (isOpenAtNarrow && !isLarge) { @@ -241,7 +239,7 @@ const _SubNavRoot = memo(({id, children, className, 'data-testid': testId, fullW
{HeadingChild &&
{HeadingChild}
} - {activeLinklabel && ( + {(activeLinklabel || isLarge) && (