Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(action-menu): action menu and menuitem accessibility omnibus #5023

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/three-taxis-hunt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@spectrum-web-components/menu": patch
---

fix(action-menu): action menu doesn't read for screen readers
54 changes: 27 additions & 27 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,23 @@ parameters:
# 3. Commit this change to the PR branch where the changes exist.
current_golden_images_hash:
type: string
default: 93ad92a2fc031db0277fee635434955e438e94a9
default: a095ed1ef88686fdb24936dad763598ac7981193
wireit_cache_name:
type: string
default: wireit
commands:
downstream:
steps:
- checkout
- restore_cache:
keys:
- v4c-dependencies-{{ arch }}-{{ checksum "yarn.lock" }}
- restore_cache:
keys:
- v4b-<< pipeline.parameters.wireit_cache_name >>-{{ arch }}-{{ checksum "package.json" }}-
# - restore_cache:
# keys:
# - v4c-dependencies-{{ arch }}-{{ checksum "yarn.lock" }}
# - restore_cache:
# keys:
# - v4b-<< pipeline.parameters.wireit_cache_name >>-{{ arch }}-{{ checksum "package.json" }}-
- run:
name: Installing Dependencies
command: |
corepack enable
yarn --immutable

command: yarn --frozen-lockfile --cache-folder ~/.cache/yarn
- save_cache:
paths:
- ~/.cache/yarn
Expand Down Expand Up @@ -88,14 +85,14 @@ commands:
if [[ $(cat count_start.txt) -eq $(cat count_end.txt) ]]; then exit 0; else exit 1; fi
# build diff review site
- run:
when: always
when: on_fail
name: Create review site
command: |
branch=$(git symbolic-ref --short HEAD)
node test/visual/review.js --branch=$branch --commit=<< pipeline.git.revision >> --system="<< parameters.regression_system >> << parameters.regression_color >> << parameters.regression_scale >> << parameters.regression_dir >>"
yarn rollup -c test/visual/rollup.config.js
- run:
when: always
when: on_fail
name: Publish review site
command: |
cp projects/documentation/content/favicon.ico test/visual
Expand Down Expand Up @@ -126,7 +123,18 @@ jobs:
executor: node

steps:
- downstream
- checkout
- restore_cache:
keys:
- v3b-dependencies-{{ arch }}-{{ checksum "yarn.lock" }}
- run:
name: Installing Dependencies
command: yarn --ignore-scripts --frozen-lockfile --cache-folder ~/.cache/yarn
- save_cache:
paths:
- ~/.cache/yarn
- .wireit
key: v3b-dependencies-{{ arch }}-{{ checksum "yarn.lock" }}
- run:
name: Define environment variable with lastest commit's message
command: |
Expand Down Expand Up @@ -203,20 +211,12 @@ jobs:

preview-docs:
executor: node

steps:
- downstream
- run:
name: Generate Custom Elements Manifest
command: yarn docs:analyze
- run:
name: Move CEM to Storybook directory
command: cp projects/documentation/custom-elements.json storybook/
- run:
name: Build documentation
command: yarn docs:build
- run:
name: Build Storybook
command: yarn storybook:build
name: Generate Docs
command: yarn docs:preview
- run: echo '/* /index.html 200' > projects/documentation/dist/_redirects
- run: |
branch=$(git symbolic-ref --short HEAD)
Expand Down Expand Up @@ -272,14 +272,14 @@ jobs:
if [[ $(cat count_start.txt) -eq $(cat count_end.txt) ]]; then exit 0; else exit 1; fi
# build diff review site
- run:
when: always
when: on_fail
name: Create review site
command: |
branch=$(git symbolic-ref --short HEAD)
node test/visual/review.js --branch=$branch --commit=<< pipeline.git.revision >> --system="hcm"
yarn rollup -c test/visual/rollup.config.js
- run:
when: always
when: on_fail
name: Publish review site
command: |
cp projects/documentation/content/favicon.ico test/visual
Expand Down
5 changes: 0 additions & 5 deletions .gitattributes

This file was deleted.

6 changes: 1 addition & 5 deletions .github/actions/setup-job/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@ description: Common setup for all jobs
runs:
using: 'composite'
steps:
- name: Enable Corepack
shell: bash
run: corepack enable

- name: Setup Node 20
uses: actions/setup-node@v4
with:
Expand All @@ -16,4 +12,4 @@ runs:

- name: Install dependencies
shell: bash
run: yarn --immutable
run: yarn --frozen-lockfile
6 changes: 3 additions & 3 deletions .github/workflows/chromatic-vrt.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@ jobs:
with:
fetch-depth: 0

- name: Setup Node 20
- name: Setup Node 18
uses: actions/setup-node@v4
with:
node-version: '20'
node-version: '18'
cache: 'yarn'
registry-url: 'https://registry.npmjs.org'

- name: Install dependencies
run: yarn --immutable
run: yarn --frozen-lockfile

- name: Publish to Chromatic
id: chromatic
Expand Down
9 changes: 0 additions & 9 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,3 @@ chromatic.log
chromatic-build-*.xml
chromatic-diagnostics*.json
chromatic.config.json

# yarn
.pnp.*
.yarn/*
!.yarn/patches
!.yarn/plugins
!.yarn/releases
!.yarn/sdks
!.yarn/versio;/
934 changes: 0 additions & 934 deletions .yarn/releases/yarn-4.6.0.cjs

This file was deleted.

5 changes: 0 additions & 5 deletions .yarnrc.yml

This file was deleted.

6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"type": "module",
"engines": {
"node": ">=20",
"yarn": ">=4.6.0"
"yarn": ">=1.16.0"
},
"scripts": {
"analyze": "lit-analyzer \"{packages,tools}/*/src/**/!(*.css).ts\"",
Expand Down Expand Up @@ -143,9 +143,9 @@
"@webcomponents/webcomponentsjs": "^2.8.0",
"alex": "^11.0.1",
"cem-plugin-module-file-extensions": "^0.0.5",
"chalk": "^5.0.1",
"chromatic": "^11.20.0",
"chromedriver": "^130.0.1",
"colors": "^1.4.0",
"common-tags": "^1.8.2",
"custom-elements-manifest": "^2.0.0",
"debounce": "^2.0.0",
Expand Down Expand Up @@ -433,5 +433,5 @@
"tools/*",
"react/*"
],
"packageManager": "yarn@4.6.0"
"packageManager": "yarn@1.22.22"
}
2 changes: 1 addition & 1 deletion packages/accordion/src/accordion-item-overrides.css
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
This file is licensed to you under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License. You may obtain a copy
of the License at http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software distributed under
the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
OF ANY KIND, either express or implied. See the License for the specific language
Expand Down
2 changes: 1 addition & 1 deletion packages/accordion/src/accordion-overrides.css
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
This file is licensed to you under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License. You may obtain a copy
of the License at http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software distributed under
the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
OF ANY KIND, either express or implied. See the License for the specific language
Expand Down
2 changes: 1 addition & 1 deletion packages/action-bar/src/action-bar-overrides.css
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
This file is licensed to you under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License. You may obtain a copy
of the License at http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software distributed under
the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
OF ANY KIND, either express or implied. See the License for the specific language
Expand Down
2 changes: 1 addition & 1 deletion packages/action-button/src/action-button-overrides.css
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
This file is licensed to you under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License. You may obtain a copy
of the License at http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software distributed under
the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
OF ANY KIND, either express or implied. See the License for the specific language
Expand Down
2 changes: 1 addition & 1 deletion packages/action-group/src/action-group-overrides.css
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
This file is licensed to you under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License. You may obtain a copy
of the License at http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software distributed under
the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
OF ANY KIND, either express or implied. See the License for the specific language
Expand Down
93 changes: 93 additions & 0 deletions packages/action-menu/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
html,
nextFrame,
oneEvent,
waitUntil,
} from '@open-wc/testing';
import { testForLitDevWarnings } from '../../../test/testing-helpers';

Expand All @@ -41,6 +42,11 @@ import type { Overlay } from '@spectrum-web-components/overlay';
import { sendKeys, setViewport } from '@web/test-runner-commands';
import { TemplateResult } from '@spectrum-web-components/base';
import { isWebKit } from '@spectrum-web-components/shared';
import {
arrowDownEvent,
arrowUpEvent,
tEvent,
} from '../../../test/testing-helpers.js';
import { SAFARI_FOCUS_RING_CLASS } from '@spectrum-web-components/picker/src/MobileController.js';

ignoreResizeObserverLoopError(before, after);
Expand Down Expand Up @@ -819,5 +825,92 @@ export const testActionMenu = (mode: 'sync' | 'async'): void => {
);
expect(handleActionMenuScroll).to.have.been.called;
});
it('handle focus and keyboard input', async () => {
const el = await fixture<Menu>(html`
<sp-action-menu>
<sp-menu-item>Deselect</sp-menu-item>
<sp-menu-item>Select Inverse</sp-menu-item>
<sp-menu-item>Feather...</sp-menu-item>
<sp-menu-item>Select and Mask...</sp-menu-item>
<sp-menu-divider></sp-menu-divider>
<sp-menu-item>Save Selection</sp-menu-item>
<sp-menu-item disabled>Make Work Path</sp-menu-item>
</sp-action-menu>
`);

await waitUntil(
() => el.childItems.length == 6,
'expected menu to manage 6 items'
);
await elementUpdated(el);

const firstItem = el.querySelector(
'sp-menu-item:nth-of-type(1)'
) as MenuItem;
const thirdToLastItem = el.querySelector(
'sp-menu-item:nth-last-of-type(3)'
) as MenuItem;
const secondToLastItem = el.querySelector(
'sp-menu-item:nth-last-of-type(2)'
) as MenuItem;

el.focus();
await elementUpdated(el);
// Activate :focus-visible
await sendKeys({ press: 'ArrowDown' });
await sendKeys({ press: 'ArrowUp' });

expect(document.activeElement === el).to.be.true;
expect(firstItem.focused).to.be.true;

el.dispatchEvent(arrowUpEvent());
el.dispatchEvent(arrowUpEvent());
el.dispatchEvent(tEvent());

expect(document.activeElement === el).to.be.true;
expect(thirdToLastItem.focused).to.be.true;

el.dispatchEvent(arrowDownEvent());

expect(document.activeElement === el).to.be.true;
expect(secondToLastItem.focused).to.be.true;
});
it('accepts Numpad keys', async function () {
if (isWebKit()) {
this.skip();
}
const el = await fixture<Menu>(html`
<sp-action-menu
selects="single"
@change=${({
target: { value },
}: Event & { target: Menu }): void => {
navigator.clipboard.writeText(value);
}}
>
<sp-menu-item>Not Selected</sp-menu-item>
<sp-menu-item selected>Selected</sp-menu-item>
<sp-menu-item id="other">Other</sp-menu-item>
</sp-action-menu>
`);

await elementUpdated(el);

const otherItem = el.querySelector('#other') as MenuItem;
otherItem.focus();
await elementUpdated(el);
await sendKeys({
press: 'ArrowDown',
});
await elementUpdated(el);
await sendKeys({
press: 'NumpadEnter',
});

await elementUpdated(el);

const clipboardText = await navigator.clipboard.readText();
expect(clipboardText).to.equal('Other');
});
});
};
2 changes: 1 addition & 1 deletion packages/alert-banner/src/alert-banner-overrides.css
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
This file is licensed to you under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License. You may obtain a copy
of the License at http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software distributed under
the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
OF ANY KIND, either express or implied. See the License for the specific language
Expand Down
2 changes: 1 addition & 1 deletion packages/alert-dialog/src/alert-dialog-overrides.css
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
This file is licensed to you under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License. You may obtain a copy
of the License at http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software distributed under
the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
OF ANY KIND, either express or implied. See the License for the specific language
Expand Down
2 changes: 1 addition & 1 deletion packages/asset/src/asset-overrides.css
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
This file is licensed to you under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License. You may obtain a copy
of the License at http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software distributed under
the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
OF ANY KIND, either express or implied. See the License for the specific language
Expand Down
Loading
Loading