From 55a9d182b875481dd5ddd0afc2a85b22978e2e9a Mon Sep 17 00:00:00 2001 From: Nicholas Boll Date: Mon, 5 Aug 2024 10:17:38 -0600 Subject: [PATCH 1/2] fix(combobox): Use correct state for aria-selected (#2849) Fixes 2860. `aria-selected` for combobox menu items. The cursor was used to determine `aria-selected` instead of the selected item. This changes the visuals of the selected vs focus state, but that issue is covered in #2828. [category:Components] Release Note: This change fixes `aria-selected` in `Combobox.Menu.Item` components, but this does change the visuals of what is considered "selected". If you have any visual tests that have a screenshot of a selected state, the visual regression will have to be updated. The same is true for DOM-based snapshot tests. `aria-selected="true"` will now be added when an item is selected and not just when the virtual cursor is on the item. If your snapshot captures this DOM state, the snapshot will have to be updated. --- cypress/integration/Autocomplete.spec.ts | 60 ++++++-- cypress/integration/Select.spec.ts | 130 ++++++++++++------ .../react/combobox/lib/ComboboxMenuItem.tsx | 2 +- 3 files changed, 134 insertions(+), 58 deletions(-) diff --git a/cypress/integration/Autocomplete.spec.ts b/cypress/integration/Autocomplete.spec.ts index 9f56743dbd..b611e16885 100644 --- a/cypress/integration/Autocomplete.spec.ts +++ b/cypress/integration/Autocomplete.spec.ts @@ -67,6 +67,10 @@ describe('Autocomplete', () => { cy.findByRole('combobox').should('not.have.attr', 'aria-activedescendant'); }); + it('should not have visual focus on any element', () => { + cy.get('[role="option"].focus').should('not.exist'); + }); + it('should not have aria-selected=true on any elements', () => { cy.get('[aria-selected=true]').should('not.exist'); }); @@ -81,12 +85,16 @@ describe('Autocomplete', () => { cy.findAllByRole('combobox').should('not.have.attr', 'aria-activedescendant'); }); + it('should not have visual focus on the first item', () => { + cy.findAllByRole('option').eq(0).should('not.have.class', 'focus'); + }); + it('should not set aria-selected to the first option', () => { cy.findAllByRole('option').eq(0).should('have.not.attr', 'aria-selected'); }); }); - context('when a value is entered', () => { + context('when "Red" is typed', () => { beforeEach(() => { cy.findByRole('combobox').type('Red', {delay: 1}); waitForAutocompleteReady(); @@ -139,8 +147,8 @@ describe('Autocomplete', () => { ); }); - it('should set aria-selected to the first option', () => { - cy.findAllByRole('option').eq(0).should('have.attr', 'aria-selected', 'true'); + it('should set visual focus to the first option', () => { + cy.findAllByRole('option').eq(0).should('have.class', 'focus'); }); context('when the user presses the enter key', () => { @@ -156,7 +164,7 @@ describe('Autocomplete', () => { cy.findByRole('listbox').should('not.exist'); }); - context('when the use hits the "2" key', () => { + context('when the user hits the "2" key', () => { beforeEach(() => { cy.findAllByRole('combobox').type('2'); waitForAutocompleteReady(); @@ -173,6 +181,10 @@ describe('Autocomplete', () => { it.skip('should change the filtered results', () => { cy.findByRole('option', {name: 'Red Apple 121'}).should('be.visible'); }); + + it('should set aria-selected to the first option', () => { + cy.findAllByRole('option').eq(0).should('have.attr', 'aria-selected', 'true'); + }); }); }); @@ -230,8 +242,12 @@ describe('Autocomplete', () => { ); }); - it('should set aria-selected to the second option', () => { - cy.findAllByRole('option').eq(1).should('have.attr', 'aria-selected', 'true'); + it('should set visual focus on the second option', () => { + cy.findAllByRole('option').eq(1).should('have.class', 'focus'); + }); + + it('should not have aria-selected=true on any elements', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); @@ -253,8 +269,12 @@ describe('Autocomplete', () => { ); }); - it('should set aria-selected to the first option', () => { - cy.findAllByRole('option').eq(0).should('have.attr', 'aria-selected', 'true'); + it('should set visual focus on the first option', () => { + cy.findAllByRole('option').eq(0).should('have.class', 'focus'); + }); + + it('should not have aria-selected=true on any elements', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); @@ -269,8 +289,12 @@ describe('Autocomplete', () => { ); }); - it('should set aria-selected to the last option', () => { - cy.findAllByRole('option').eq(3).should('have.attr', 'aria-selected', 'true'); + it('should set visual focus on the last option', () => { + cy.findAllByRole('option').eq(3).should('have.class', 'focus'); + }); + + it('should not have aria-selected=true on any elements', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); @@ -285,8 +309,12 @@ describe('Autocomplete', () => { ); }); - it('should set aria-selected to the third option', () => { - cy.findAllByRole('option').eq(2).should('have.attr', 'aria-selected', 'true'); + it('should set visual focus on the third option', () => { + cy.findAllByRole('option').eq(2).should('have.class', 'focus'); + }); + + it('should not have aria-selected=true on any elements', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); @@ -300,8 +328,12 @@ describe('Autocomplete', () => { ); }); - it('should set aria-selected to the first option', () => { - cy.findAllByRole('option').eq(0).should('have.attr', 'aria-selected', 'true'); + it('should set visual focus on the first option', () => { + cy.findAllByRole('option').eq(0).should('have.class', 'focus'); + }); + + it('should not have aria-selected=true on any elements', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); }); diff --git a/cypress/integration/Select.spec.ts b/cypress/integration/Select.spec.ts index e9a6ec2459..b8f761441c 100644 --- a/cypress/integration/Select.spec.ts +++ b/cypress/integration/Select.spec.ts @@ -115,11 +115,11 @@ describe('Select', () => { context('the menu', () => { it('should scroll so that the "San Francisco (United States)" option is fully visible', () => { - cy.findByText('San Francisco (United States)').should( - 'have.attr', - 'aria-selected', - 'true' - ); + cy.findByText('San Francisco (United States)').should('have.class', 'focus'); + }); + + it('should not have any item selected', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); }); @@ -132,11 +132,11 @@ describe('Select', () => { context('the menu', () => { it('should scroll so that the "San Mateo (United States)" option is fully visible', () => { - cy.findByText('San Mateo (United States)').should( - 'have.attr', - 'aria-selected', - 'true' - ); + cy.findByText('San Mateo (United States)').should('have.class', 'focus'); + }); + + it('should not have any item selected', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); }); @@ -149,11 +149,11 @@ describe('Select', () => { context('the menu', () => { it('should scroll so that the "Dallas (United States)" option is fully visible', () => { - cy.findByText('Dallas (United States)').should( - 'have.attr', - 'aria-selected', - 'true' - ); + cy.findByText('Dallas (United States)').should('have.class', 'focus'); + }); + + it('should not have any item selected', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); }); @@ -170,11 +170,11 @@ describe('Select', () => { context('the menu', () => { it('should set assistive focus to the "San Francisco (United States)" option', () => { - cy.findByText('San Francisco (United States)').should( - 'have.attr', - 'aria-selected', - 'true' - ); + cy.findByText('San Francisco (United States)').should('have.class', 'focus'); + }); + + it('should not have any item selected', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); }); @@ -186,11 +186,11 @@ describe('Select', () => { context('the select input', () => { it('should set assistive focus to the "San Francisco (United States)" option', () => { - cy.findByText('San Francisco (United States)').should( - 'have.attr', - 'aria-selected', - 'true' - ); + cy.findByText('San Francisco (United States)').should('have.class', 'focus'); + }); + + it('should not have any item selected', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); }); @@ -203,11 +203,11 @@ describe('Select', () => { context('the select input', () => { it('should set assistive focus to the "San Mateo (United States)" option', () => { - cy.findByText('San Mateo (United States)').should( - 'have.attr', - 'aria-selected', - 'true' - ); + cy.findByText('San Mateo (United States)').should('have.class', 'focus'); + }); + + it('should not have any item selected', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); }); @@ -231,6 +231,10 @@ describe('Select', () => { 'true' ); }); + + it('should set assistive focus to the "Dallas (United States)" option', () => { + cy.findByText('Dallas (United States)').should('have.class', 'focus'); + }); }); }); } @@ -266,7 +270,11 @@ describe('Select', () => { context('the menu', () => { it('should set assistive focus to the first option ("E-mail")', () => { - cy.findAllByRole('option').eq(0).should('have.attr', 'aria-selected', 'true'); + cy.findAllByRole('option').eq(0).should('have.class', 'focus'); + }); + + it('should not have any item selected', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); @@ -277,7 +285,11 @@ describe('Select', () => { context('the menu', () => { it('should set assistive focus to the second option ("Phone")', () => { - cy.findAllByRole('option').eq(1).should('have.attr', 'aria-selected', 'true'); + cy.findAllByRole('option').eq(1).should('have.class', 'focus'); + }); + + it('should not have any item selected', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); @@ -292,6 +304,10 @@ describe('Select', () => { cy.findByRole('combobox').should('not.have.attr', 'aria-activedescendant'); }); + it('should not have any item selected', () => { + cy.get('[aria-selected=true]').should('not.exist'); + }); + context('when the menu is re-opened AFTER it has fully closed', () => { beforeEach(() => { // Wait for menu to fully close before we open it again (so we @@ -303,7 +319,11 @@ describe('Select', () => { context('the menu', () => { it('should set assistive focus to the second option ("Phone") that is where the cursor was', () => { - cy.findAllByRole('option').eq(1).should('have.attr', 'aria-selected', 'true'); + cy.findByRole('option', {name: 'Phone'}).should('have.class', 'focus'); + }); + + it('should not have any item selected', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); }); @@ -336,7 +356,11 @@ describe('Select', () => { context('the menu', () => { it('should set assistive focus to second enabled option ("Phone")', () => { - cy.findAllByRole('option').eq(1).should('have.attr', 'aria-selected', 'true'); + cy.findAllByRole('option').eq(1).should('have.class', 'focus'); + }); + + it('should not have any item selected', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); @@ -347,7 +371,11 @@ describe('Select', () => { context('the menu', () => { it('should set assistive focus to the fourth option down ("Mail") since focus will have skipped one disabled option ("Fax")', () => { - cy.findAllByRole('option').eq(3).should('have.attr', 'aria-selected', 'true'); + cy.findByRole('option', {name: 'Mail'}).should('have.class', 'focus'); + }); + + it('should not have any item selected', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); }); @@ -409,9 +437,13 @@ describe('Select', () => { }); context('when Boulder is reached via the arrow key', () => { it('should show Boulder (United States)', () => { - cy.findByText('Boulder (United States)').should('have.attr', 'aria-selected', 'true'); + cy.findByText('Boulder (United States)').should('have.class', 'focus'); cy.findByText('Boulder (United States)').should('be.visible'); }); + + it('should not have any item selected', () => { + cy.get('[aria-selected=true]').should('not.exist'); + }); }); }); }); @@ -461,8 +493,12 @@ describe('Select', () => { }); context('the first option ("E-Mail")', () => { - it('should have an aria-selected attribute set to "true"', () => { - cy.findAllByRole('option').eq(0).should('have.attr', 'aria-selected', 'true'); + it('should set accessible focus on the "E-Mail" option', () => { + cy.findAllByRole('option').eq(0).should('have.class', 'focus'); + }); + + it('should not have any item selected', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); @@ -495,6 +531,10 @@ describe('Select', () => { context('the menu', () => { it('should set assistive focus to the "Phone" option', () => { + cy.findByText('Phone').should('have.class', 'focus'); + }); + + it('should set assistive selection on the "Phone" option', () => { cy.findByText('Phone').should('have.attr', 'aria-selected', 'true'); }); }); @@ -526,7 +566,7 @@ describe('Select', () => { context('the menu', () => { it('should set assistive focus to the "Phone" option', () => { - cy.findAllByRole('option').eq(1).should('have.attr', 'aria-selected', 'true'); + cy.findByRole('option', {name: 'Phone'}).should('have.class', 'focus'); }); }); @@ -537,14 +577,18 @@ describe('Select', () => { context('the menu', () => { it('should set assistive focus to the "Mail" option and skip disabled fax', () => { - cy.findAllByRole('option').eq(3).should('have.attr', 'aria-selected', 'true'); + cy.findByRole('option', {name: 'Mail'}).should('have.class', 'focus'); + }); + + it('should not have any item selected', () => { + cy.get('[aria-selected=true]').should('not.exist'); }); }); }); }); }); - context('when the enter key is pressed', () => { + context('when the down arrow key is pressed', () => { beforeEach(() => { cy.findByRole('combobox').focus().realType('{downarrow}'); }); @@ -554,8 +598,8 @@ describe('Select', () => { cy.findByRole('listbox').should('be.visible'); }); - it('should have E-Mail selected', () => { - cy.findAllByRole('option').eq(0).should('have.attr', 'aria-selected', 'true'); + it('should set accessible focus on the "E-Mail" option', () => { + cy.findAllByRole('option').eq(0).should('have.class', 'focus'); }); }); diff --git a/modules/react/combobox/lib/ComboboxMenuItem.tsx b/modules/react/combobox/lib/ComboboxMenuItem.tsx index ce38056470..9717627dd2 100644 --- a/modules/react/combobox/lib/ComboboxMenuItem.tsx +++ b/modules/react/combobox/lib/ComboboxMenuItem.tsx @@ -44,7 +44,7 @@ export const useComboboxMenuItem = composeHooks( event.preventDefault(); }; - const selected = model.state.cursorId === id; + const selected = model.state.selectedIds[0] === id; return { role: 'option', From 419f977a18c6952dca36a9490b4eb2c3de9d1309 Mon Sep 17 00:00:00 2001 From: alanbsmith Date: Mon, 5 Aug 2024 16:18:15 +0000 Subject: [PATCH 2/2] chore: Release v10.3.50 [skip release] --- CHANGELOG.md | 8 ++++++++ lerna.json | 2 +- modules/codemod/package.json | 2 +- modules/docs/package.json | 10 +++++----- modules/labs-react/package.json | 4 ++-- modules/popup-stack/package.json | 2 +- modules/preview-react/package.json | 6 +++--- modules/react-fonts/package.json | 2 +- modules/react/package.json | 6 +++--- modules/styling-transform/package.json | 4 ++-- modules/styling/package.json | 4 ++-- 11 files changed, 29 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 706f030179..84e162de1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,14 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +## [v10.3.50](https://github.com/Workday/canvas-kit/releases/tag/v10.3.50) (2024-08-05) + +### Components + +- fix(combobox): Use correct state for aria-selected ([#2849](https://github.com/Workday/canvas-kit/pull/2849)) ([@NicholasBoll](https://github.com/NicholasBoll)) + This change fixes `aria-selected` in `Combobox.Menu.Item` components, but this does change the visuals of what is considered "selected". If you have any visual tests that have a screenshot of a selected state, the visual regression will have to be updated. The same is true for DOM-based snapshot tests. `aria-selected="true"` will now be added when an item is selected and not just when the virtual cursor is on the item. If your snapshot captures this DOM state, the snapshot will have to be updated. + + ## [v10.3.49](https://github.com/Workday/canvas-kit/releases/tag/v10.3.49) (2024-07-29) ### Components diff --git a/lerna.json b/lerna.json index 948de85fac..ddab71363f 100644 --- a/lerna.json +++ b/lerna.json @@ -2,7 +2,7 @@ "packages": [ "modules/**" ], - "version": "10.3.49", + "version": "10.3.50", "npmClient": "yarn", "useWorkspaces": true, "command": { diff --git a/modules/codemod/package.json b/modules/codemod/package.json index 1c9eae7ac3..c2733f8882 100644 --- a/modules/codemod/package.json +++ b/modules/codemod/package.json @@ -2,7 +2,7 @@ "name": "@workday/canvas-kit-codemod", "author": "Workday, Inc. (https://www.workday.com)", "license": "Apache-2.0", - "version": "10.3.49", + "version": "10.3.50", "description": "A collection of codemods for use on Workday Canvas Kit packages.", "main": "dist/es6/index.js", "sideEffects": false, diff --git a/modules/docs/package.json b/modules/docs/package.json index 9ebd6633b1..a7cd345776 100644 --- a/modules/docs/package.json +++ b/modules/docs/package.json @@ -1,6 +1,6 @@ { "name": "@workday/canvas-kit-docs", - "version": "10.3.49", + "version": "10.3.50", "description": "Documentation components of Canvas Kit components", "author": "Workday, Inc. (https://www.workday.com)", "license": "Apache-2.0", @@ -44,10 +44,10 @@ "dependencies": { "@emotion/styled": "^11.6.0", "@storybook/csf": "0.0.1", - "@workday/canvas-kit-labs-react": "^10.3.49", - "@workday/canvas-kit-preview-react": "^10.3.49", - "@workday/canvas-kit-react": "^10.3.49", - "@workday/canvas-kit-styling": "^10.3.49", + "@workday/canvas-kit-labs-react": "^10.3.50", + "@workday/canvas-kit-preview-react": "^10.3.50", + "@workday/canvas-kit-react": "^10.3.50", + "@workday/canvas-kit-styling": "^10.3.50", "@workday/canvas-system-icons-web": "^3.0.0", "@workday/canvas-tokens-web": "^1.0.0", "markdown-to-jsx": "^6.10.3", diff --git a/modules/labs-react/package.json b/modules/labs-react/package.json index 61e7c5bfca..dcf2497b1a 100644 --- a/modules/labs-react/package.json +++ b/modules/labs-react/package.json @@ -1,6 +1,6 @@ { "name": "@workday/canvas-kit-labs-react", - "version": "10.3.49", + "version": "10.3.50", "description": "Canvas Kit Labs is an incubator for new and experimental components. Since we have a rather rigorous process for getting components in at a production level, it can be valuable to make them available earlier while we continuously iterate on the API/functionality. The Labs modules allow us to do that as needed.", "author": "Workday, Inc. (https://www.workday.com)", "license": "Apache-2.0", @@ -46,7 +46,7 @@ "dependencies": { "@emotion/react": "^11.7.1", "@emotion/styled": "^11.6.0", - "@workday/canvas-kit-react": "^10.3.49", + "@workday/canvas-kit-react": "^10.3.50", "@workday/canvas-system-icons-web": "^3.0.0", "@workday/design-assets-types": "^0.2.8", "chroma-js": "^2.1.0", diff --git a/modules/popup-stack/package.json b/modules/popup-stack/package.json index 4664d49008..27cd3a5601 100644 --- a/modules/popup-stack/package.json +++ b/modules/popup-stack/package.json @@ -1,6 +1,6 @@ { "name": "@workday/canvas-kit-popup-stack", - "version": "10.3.49", + "version": "10.3.50", "description": "Stack for managing popup UIs to coordinate global concerns like escape key handling and rendering order", "author": "Workday, Inc. (https://www.workday.com)", "license": "Apache-2.0", diff --git a/modules/preview-react/package.json b/modules/preview-react/package.json index 73e0a806b5..5e0c83756c 100644 --- a/modules/preview-react/package.json +++ b/modules/preview-react/package.json @@ -1,6 +1,6 @@ { "name": "@workday/canvas-kit-preview-react", - "version": "10.3.49", + "version": "10.3.50", "description": "Canvas Kit Preview is made up of components that have the full design and a11y review, are part of the DS ecosystem and are approved for use in product. The API's could be subject to change, but not without strong communication and migration strategies.", "author": "Workday, Inc. (https://www.workday.com)", "license": "Apache-2.0", @@ -47,8 +47,8 @@ "@emotion/is-prop-valid": "^1.1.1", "@emotion/react": "^11.7.1", "@emotion/styled": "^11.6.0", - "@workday/canvas-kit-react": "^10.3.49", - "@workday/canvas-kit-styling": "^10.3.49", + "@workday/canvas-kit-react": "^10.3.50", + "@workday/canvas-kit-styling": "^10.3.50", "@workday/canvas-system-icons-web": "^3.0.0", "@workday/canvas-tokens-web": "^1.0.0", "@workday/design-assets-types": "^0.2.8" diff --git a/modules/react-fonts/package.json b/modules/react-fonts/package.json index 7cbb79ed09..9b022f154b 100644 --- a/modules/react-fonts/package.json +++ b/modules/react-fonts/package.json @@ -1,6 +1,6 @@ { "name": "@workday/canvas-kit-react-fonts", - "version": "10.3.49", + "version": "10.3.50", "description": "Fonts for canvas-kit-react", "author": "Workday, Inc. (https://www.workday.com)", "license": "Apache-2.0", diff --git a/modules/react/package.json b/modules/react/package.json index 77a677cc77..f335685e21 100644 --- a/modules/react/package.json +++ b/modules/react/package.json @@ -1,6 +1,6 @@ { "name": "@workday/canvas-kit-react", - "version": "10.3.49", + "version": "10.3.50", "description": "The parent module that contains all Workday Canvas Kit React components", "author": "Workday, Inc. (https://www.workday.com)", "license": "Apache-2.0", @@ -49,8 +49,8 @@ "@emotion/styled": "^11.6.0", "@popperjs/core": "^2.5.4", "@workday/canvas-colors-web": "^2.0.0", - "@workday/canvas-kit-popup-stack": "^10.3.49", - "@workday/canvas-kit-styling": "^10.3.49", + "@workday/canvas-kit-popup-stack": "^10.3.50", + "@workday/canvas-kit-styling": "^10.3.50", "@workday/canvas-system-icons-web": "^3.0.0", "@workday/canvas-tokens-web": "^1.0.0", "@workday/design-assets-types": "^0.2.8", diff --git a/modules/styling-transform/package.json b/modules/styling-transform/package.json index c120596f44..a99e22edbc 100644 --- a/modules/styling-transform/package.json +++ b/modules/styling-transform/package.json @@ -1,6 +1,6 @@ { "name": "@workday/canvas-kit-styling-transform", - "version": "10.3.49", + "version": "10.3.50", "description": "The custom CSS in JS solution that takes JS styles and turns them into static CSS", "author": "Workday, Inc. (https://www.workday.com)", "license": "Apache-2.0", @@ -34,7 +34,7 @@ ], "dependencies": { "@emotion/serialize": "^1.0.2", - "@workday/canvas-kit-styling": "^10.3.49", + "@workday/canvas-kit-styling": "^10.3.50", "stylis": "4.0.13", "typescript": "4.2" }, diff --git a/modules/styling/package.json b/modules/styling/package.json index 25004961ea..4a450074bf 100644 --- a/modules/styling/package.json +++ b/modules/styling/package.json @@ -1,6 +1,6 @@ { "name": "@workday/canvas-kit-styling", - "version": "10.3.49", + "version": "10.3.50", "description": "The custom CSS in JS solution that takes JS styles and turns them into static CSS", "author": "Workday, Inc. (https://www.workday.com)", "license": "Apache-2.0", @@ -53,7 +53,7 @@ "@emotion/react": "^11.7.1", "@emotion/serialize": "^1.0.2", "@emotion/styled": "^11.6.0", - "@workday/canvas-kit-react": "^10.3.49", + "@workday/canvas-kit-react": "^10.3.50", "@workday/canvas-tokens-web": "^1.0.0", "typescript": "4.2" }