From 320804069da59a15dc376c8f0b18c324a1d1e08a Mon Sep 17 00:00:00 2001 From: Brian Ferry Date: Thu, 4 May 2023 09:53:49 -0400 Subject: [PATCH 01/14] fix(accordion): adding RTI to accordion, adding headers, removing old code --- elements/pf-accordion/BaseAccordion.ts | 76 +++++++------------------- 1 file changed, 21 insertions(+), 55 deletions(-) diff --git a/elements/pf-accordion/BaseAccordion.ts b/elements/pf-accordion/BaseAccordion.ts index 294078acaf..c7616c94b4 100644 --- a/elements/pf-accordion/BaseAccordion.ts +++ b/elements/pf-accordion/BaseAccordion.ts @@ -11,6 +11,8 @@ import { Logger } from '@patternfly/pfe-core/controllers/logger.js'; import { AccordionHeaderChangeEvent, BaseAccordionHeader } from './BaseAccordionHeader.js'; import { BaseAccordionPanel } from './BaseAccordionPanel.js'; +import { RovingTabindexController } from '@patternfly/pfe-core/controllers/roving-tabindex-controller.js'; + import style from './BaseAccordion.css'; const CSS_TIMING_UNITS_RE = /^[0-9.]+(?[a-zA-Z]+)/g; @@ -48,6 +50,8 @@ export abstract class BaseAccordion extends LitElement { return target instanceof BaseAccordionPanel; } + #headerIndex = new RovingTabindexController(this); + /** * Sets and reflects the currently expanded accordion 0-based indexes. * Use commas to separate multiple indexes. @@ -78,6 +82,12 @@ export abstract class BaseAccordion extends LitElement { return this.#allPanels(); } + get activeHeader() { + const { headers } = this; + const index = headers.findIndex(header => header.matches(':focus,:focus-within')); + return headers.at(index); + } + protected expandedSets = new Set(); #logger = new Logger(this); @@ -103,9 +113,12 @@ export abstract class BaseAccordion extends LitElement { connectedCallback() { super.connectedCallback(); this.addEventListener('change', this.#onChange as EventListener); - this.addEventListener('keydown', this.#onKeydown); + // Add this to fix the RTI activeIndex event issue + // this.addEventListener('focusin', this.#updateActiveHeader as EventListener); + this.#headerIndex.initItems(this.headers); this.#mo.observe(this, { childList: true }); this.#init(); + this.requestUpdate(); } render(): TemplateResult { @@ -237,6 +250,13 @@ export abstract class BaseAccordion extends LitElement { } } + #updateActiveHeader() { + const { activeHeader } = this; + if (activeHeader) { + this.#headerIndex.updateActiveItem(activeHeader); + } + } + #onChange(event: AccordionHeaderChangeEvent) { if (this.classList.contains('animating')) { return; @@ -251,40 +271,6 @@ export abstract class BaseAccordion extends LitElement { } } - /** - * @see https://www.w3.org/TR/wai-aria-practices/#accordion - */ - async #onKeydown(evt: KeyboardEvent) { - const currentHeader = evt.target as Element; - - if (!BaseAccordion.isHeader(currentHeader)) { - return; - } - - let newHeader: BaseAccordionHeader | undefined; - - switch (evt.key) { - case 'ArrowDown': - evt.preventDefault(); - newHeader = this.#nextHeader(); - break; - case 'ArrowUp': - evt.preventDefault(); - newHeader = this.#previousHeader(); - break; - case 'Home': - evt.preventDefault(); - newHeader = this.#firstHeader(); - break; - case 'End': - evt.preventDefault(); - newHeader = this.#lastHeader(); - break; - } - - newHeader?.focus?.(); - } - #allHeaders(accordion: BaseAccordion = this): BaseAccordionHeader[] { return Array.from(accordion.children).filter(BaseAccordion.isHeader); } @@ -293,26 +279,6 @@ export abstract class BaseAccordion extends LitElement { return Array.from(accordion.children).filter(BaseAccordion.isPanel); } - #previousHeader() { - const { headers } = this; - const newIndex = headers.findIndex(header => header.matches(':focus,:focus-within')) - 1; - return headers[(newIndex + headers.length) % headers.length]; - } - - #nextHeader() { - const { headers } = this; - const newIndex = headers.findIndex(header => header.matches(':focus,:focus-within')) + 1; - return headers[newIndex % headers.length]; - } - - #firstHeader() { - return this.headers.at(0); - } - - #lastHeader() { - return this.headers.at(-1); - } - #getIndex(el: Element | null) { if (BaseAccordion.isHeader(el)) { return this.headers.findIndex(header => header.id === el.id); From e91538da70c1eb1f77ff8a1917cdadc02ba31baf Mon Sep 17 00:00:00 2001 From: Brian Ferry Date: Mon, 8 May 2023 09:25:09 -0400 Subject: [PATCH 02/14] fix(accordion): adding RTI index after initialized --- elements/pf-accordion/BaseAccordion.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/elements/pf-accordion/BaseAccordion.ts b/elements/pf-accordion/BaseAccordion.ts index c7616c94b4..79954007ae 100644 --- a/elements/pf-accordion/BaseAccordion.ts +++ b/elements/pf-accordion/BaseAccordion.ts @@ -148,6 +148,8 @@ export abstract class BaseAccordion extends LitElement { */ async #init() { this.#initialized ||= !!await this.updateComplete; + // Event listener to the accordion header after the accordion has been initialized to add the roving tabindex + this.addEventListener('focusin', this.#updateActiveHeader as EventListener); this.updateAccessibility(); } From c203b475ea890fbfc493f7daedb7f36c5e6896ab Mon Sep 17 00:00:00 2001 From: Brian Ferry Date: Mon, 8 May 2023 09:28:27 -0400 Subject: [PATCH 03/14] fix(accordion): removing older code for rti fix --- elements/pf-accordion/BaseAccordion.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/elements/pf-accordion/BaseAccordion.ts b/elements/pf-accordion/BaseAccordion.ts index 79954007ae..1dc1a5fefa 100644 --- a/elements/pf-accordion/BaseAccordion.ts +++ b/elements/pf-accordion/BaseAccordion.ts @@ -113,12 +113,9 @@ export abstract class BaseAccordion extends LitElement { connectedCallback() { super.connectedCallback(); this.addEventListener('change', this.#onChange as EventListener); - // Add this to fix the RTI activeIndex event issue - // this.addEventListener('focusin', this.#updateActiveHeader as EventListener); this.#headerIndex.initItems(this.headers); this.#mo.observe(this, { childList: true }); this.#init(); - this.requestUpdate(); } render(): TemplateResult { From ec290dfbfa37e43ed97ad69ab2f69f575ec98e66 Mon Sep 17 00:00:00 2001 From: Brian Ferry Date: Mon, 8 May 2023 09:35:19 -0400 Subject: [PATCH 04/14] chore: adding changeset --- .changeset/empty-trainers-tickle.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/empty-trainers-tickle.md diff --git a/.changeset/empty-trainers-tickle.md b/.changeset/empty-trainers-tickle.md new file mode 100644 index 0000000000..e997131227 --- /dev/null +++ b/.changeset/empty-trainers-tickle.md @@ -0,0 +1,6 @@ +--- +"@patternfly/elements": patch +--- + + +**pf-accordion**: Fixing keyboard navigation inside of nested accordions. \ No newline at end of file From 370b80a0a804b9c4befa5f3e6ae0381601eb7bfa Mon Sep 17 00:00:00 2001 From: Nikki Massaro Kauffman Date: Mon, 8 May 2023 16:32:51 -0400 Subject: [PATCH 05/14] Merge branch `fix/accordion-rovingtabindex-controller` into `fix/accordion-rovingtabindex-controller` --- .changeset/pf-tabs-id-cc.md | 4 ++ .changeset/strong-donkeys-care.md | 5 ++ elements/pf-accordion/BaseAccordion.ts | 76 +++++++++++++++++++------- elements/pf-tabs/BaseTabs.ts | 3 +- tools/pfe-tools/dev-server/config.ts | 16 +++++- 5 files changed, 79 insertions(+), 25 deletions(-) create mode 100644 .changeset/pf-tabs-id-cc.md create mode 100644 .changeset/strong-donkeys-care.md diff --git a/.changeset/pf-tabs-id-cc.md b/.changeset/pf-tabs-id-cc.md new file mode 100644 index 0000000000..c655949e9e --- /dev/null +++ b/.changeset/pf-tabs-id-cc.md @@ -0,0 +1,4 @@ +--- +"@patternfly/elements": "patch" +--- +``: prevent error when using in certain javascript frameworks diff --git a/.changeset/strong-donkeys-care.md b/.changeset/strong-donkeys-care.md new file mode 100644 index 0000000000..53ea0eeee3 --- /dev/null +++ b/.changeset/strong-donkeys-care.md @@ -0,0 +1,5 @@ +--- +"@patternfly/pfe-tools": patch +--- + +**Dev Server**: fixes `*-lightdom.css` support in config diff --git a/elements/pf-accordion/BaseAccordion.ts b/elements/pf-accordion/BaseAccordion.ts index c7616c94b4..294078acaf 100644 --- a/elements/pf-accordion/BaseAccordion.ts +++ b/elements/pf-accordion/BaseAccordion.ts @@ -11,8 +11,6 @@ import { Logger } from '@patternfly/pfe-core/controllers/logger.js'; import { AccordionHeaderChangeEvent, BaseAccordionHeader } from './BaseAccordionHeader.js'; import { BaseAccordionPanel } from './BaseAccordionPanel.js'; -import { RovingTabindexController } from '@patternfly/pfe-core/controllers/roving-tabindex-controller.js'; - import style from './BaseAccordion.css'; const CSS_TIMING_UNITS_RE = /^[0-9.]+(?[a-zA-Z]+)/g; @@ -50,8 +48,6 @@ export abstract class BaseAccordion extends LitElement { return target instanceof BaseAccordionPanel; } - #headerIndex = new RovingTabindexController(this); - /** * Sets and reflects the currently expanded accordion 0-based indexes. * Use commas to separate multiple indexes. @@ -82,12 +78,6 @@ export abstract class BaseAccordion extends LitElement { return this.#allPanels(); } - get activeHeader() { - const { headers } = this; - const index = headers.findIndex(header => header.matches(':focus,:focus-within')); - return headers.at(index); - } - protected expandedSets = new Set(); #logger = new Logger(this); @@ -113,12 +103,9 @@ export abstract class BaseAccordion extends LitElement { connectedCallback() { super.connectedCallback(); this.addEventListener('change', this.#onChange as EventListener); - // Add this to fix the RTI activeIndex event issue - // this.addEventListener('focusin', this.#updateActiveHeader as EventListener); - this.#headerIndex.initItems(this.headers); + this.addEventListener('keydown', this.#onKeydown); this.#mo.observe(this, { childList: true }); this.#init(); - this.requestUpdate(); } render(): TemplateResult { @@ -250,13 +237,6 @@ export abstract class BaseAccordion extends LitElement { } } - #updateActiveHeader() { - const { activeHeader } = this; - if (activeHeader) { - this.#headerIndex.updateActiveItem(activeHeader); - } - } - #onChange(event: AccordionHeaderChangeEvent) { if (this.classList.contains('animating')) { return; @@ -271,6 +251,40 @@ export abstract class BaseAccordion extends LitElement { } } + /** + * @see https://www.w3.org/TR/wai-aria-practices/#accordion + */ + async #onKeydown(evt: KeyboardEvent) { + const currentHeader = evt.target as Element; + + if (!BaseAccordion.isHeader(currentHeader)) { + return; + } + + let newHeader: BaseAccordionHeader | undefined; + + switch (evt.key) { + case 'ArrowDown': + evt.preventDefault(); + newHeader = this.#nextHeader(); + break; + case 'ArrowUp': + evt.preventDefault(); + newHeader = this.#previousHeader(); + break; + case 'Home': + evt.preventDefault(); + newHeader = this.#firstHeader(); + break; + case 'End': + evt.preventDefault(); + newHeader = this.#lastHeader(); + break; + } + + newHeader?.focus?.(); + } + #allHeaders(accordion: BaseAccordion = this): BaseAccordionHeader[] { return Array.from(accordion.children).filter(BaseAccordion.isHeader); } @@ -279,6 +293,26 @@ export abstract class BaseAccordion extends LitElement { return Array.from(accordion.children).filter(BaseAccordion.isPanel); } + #previousHeader() { + const { headers } = this; + const newIndex = headers.findIndex(header => header.matches(':focus,:focus-within')) - 1; + return headers[(newIndex + headers.length) % headers.length]; + } + + #nextHeader() { + const { headers } = this; + const newIndex = headers.findIndex(header => header.matches(':focus,:focus-within')) + 1; + return headers[newIndex % headers.length]; + } + + #firstHeader() { + return this.headers.at(0); + } + + #lastHeader() { + return this.headers.at(-1); + } + #getIndex(el: Element | null) { if (BaseAccordion.isHeader(el)) { return this.headers.findIndex(header => header.id === el.id); diff --git a/elements/pf-tabs/BaseTabs.ts b/elements/pf-tabs/BaseTabs.ts index ead02de97a..88267bd0f6 100644 --- a/elements/pf-tabs/BaseTabs.ts +++ b/elements/pf-tabs/BaseTabs.ts @@ -73,8 +73,6 @@ export abstract class BaseTabs extends LitElement { #activeIndex = 0; - id: string = this.id || getRandomId(this.localName); - /** * Tab activation * Tabs can be either [automatic](https://w3c.github.io/aria-practices/examples/tabs/tabs-automatic.html) activated @@ -139,6 +137,7 @@ export abstract class BaseTabs extends LitElement { override connectedCallback() { super.connectedCallback(); + this.id ||= getRandomId(this.localName); this.addEventListener('expand', this.#onTabExpand); BaseTabs.#instances.add(this); } diff --git a/tools/pfe-tools/dev-server/config.ts b/tools/pfe-tools/dev-server/config.ts index f8e77f1461..45f1fe5234 100644 --- a/tools/pfe-tools/dev-server/config.ts +++ b/tools/pfe-tools/dev-server/config.ts @@ -116,11 +116,18 @@ function pfeDevServerPlugin(options: PfeDevServerInternalConfig): Plugin { const { element, fileName } = ctx.params; ctx.redirect(`/${elementsDir}/${element}/${fileName}.ts`); }) + // Redirect `components/pf-jazz-hands|jazz-hands/demo/*-lightdom.css` to `components/pf-jazz-hands/*-lightdom.css` // Redirect `components/jazz-hands/demo/*.js|css` to `components/pf-jazz-hands/demo/*.js|css` .get(`/${componentSubpath}/:element/demo/:demoSubDir?/:fileName.:ext`, async (ctx, next) => { const { element, fileName, ext } = ctx.params; + let prefixedElement = element; if (!element.includes(tagPrefix)) { - ctx.redirect(`/${elementsDir}/${tagPrefix}-${element}/demo/${fileName}.${ext}`); + prefixedElement = `${tagPrefix}-${element}`; + } + if (fileName.includes('-lightdom') && ext === 'css') { + ctx.redirect(`/${elementsDir}/${prefixedElement}/${fileName}.${ext}`); + } else if (!element.includes(tagPrefix)) { + ctx.redirect(`/${elementsDir}/${prefixedElement}/demo/${fileName}.${ext}`); } else { return next(); } @@ -152,7 +159,12 @@ function normalizeOptions(options?: PfeDevServerConfigOptions): PfeDevServerInte config.site = { ...config.site, ...options?.site ?? {} }; config.loadDemo ??= true; config.watchFiles ??= '{elements,core}/**/*.{css,html}'; - config.litcssOptions ??= { include: /\.css$/, exclude: /((fonts|demo)|(demo\/.*))\.css$/ }; + + config.litcssOptions ??= { + include: /\.css$/, + exclude: /(((fonts|demo)|(demo\/.*))\.css$)|(.*(-lightdom.css$))/ + }; + return config as PfeDevServerInternalConfig; } From 6f492cd834d59afd8661f01f1a53f13c2cf0b9c2 Mon Sep 17 00:00:00 2001 From: Nikki Massaro Kauffman Date: Mon, 8 May 2023 16:34:50 -0400 Subject: [PATCH 06/14] fix(core): ensured rti arrow key listeners only apply to focusable items --- core/pfe-core/controllers/roving-tabindex-controller.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/pfe-core/controllers/roving-tabindex-controller.ts b/core/pfe-core/controllers/roving-tabindex-controller.ts index 8f2275214f..c576ba07d0 100644 --- a/core/pfe-core/controllers/roving-tabindex-controller.ts +++ b/core/pfe-core/controllers/roving-tabindex-controller.ts @@ -93,10 +93,11 @@ export class RovingTabindexController< * handles keyboard navigation */ #onKeydown = (event: KeyboardEvent) => { - if (event.ctrlKey || event.altKey || event.metaKey || this.#focusableItems.length < 1) { + const composed = event.composedPath(); + const targetIsFocusable = () => this.#focusableItems.filter(x=>composed.includes(x))?.length > 0; + if (event.ctrlKey || event.altKey || event.metaKey || this.#focusableItems.length < 1 || !targetIsFocusable()) { return; } - const item = this.activeItem; let shouldPreventDefault = false; const horizontalOnly = From 5a78363f2c4629ebd7991a8c86290c219eb3dfa8 Mon Sep 17 00:00:00 2001 From: Nikki Massaro Kauffman Date: Mon, 8 May 2023 16:38:14 -0400 Subject: [PATCH 07/14] fix(core): ensured rti error key listeners only apply to focusable items --- .changeset/brown-shirts-think.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/brown-shirts-think.md diff --git a/.changeset/brown-shirts-think.md b/.changeset/brown-shirts-think.md new file mode 100644 index 0000000000..4c31a42755 --- /dev/null +++ b/.changeset/brown-shirts-think.md @@ -0,0 +1,5 @@ +--- +"@patternfly/pfe-core": patch +--- + +**roving-tabindex-controller**: fixes arrow keydown event listeners \ No newline at end of file From c8ddafb3749a80eb74cae74460fb93581fea2e25 Mon Sep 17 00:00:00 2001 From: Brian Ferry Date: Tue, 9 May 2023 10:08:44 -0400 Subject: [PATCH 08/14] fix(accordion): adding init items on mutation observer for RTI, removing tab tests --- elements/pf-accordion/BaseAccordion.ts | 19 +++++++++++++- .../pf-accordion/test/pf-accordion.spec.ts | 25 ++----------------- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/elements/pf-accordion/BaseAccordion.ts b/elements/pf-accordion/BaseAccordion.ts index 1e4d055382..10906c7ccf 100644 --- a/elements/pf-accordion/BaseAccordion.ts +++ b/elements/pf-accordion/BaseAccordion.ts @@ -11,6 +11,8 @@ import { Logger } from '@patternfly/pfe-core/controllers/logger.js'; import { AccordionHeaderChangeEvent, BaseAccordionHeader } from './BaseAccordionHeader.js'; import { BaseAccordionPanel } from './BaseAccordionPanel.js'; +import { RovingTabindexController } from '@patternfly/pfe-core/controllers/roving-tabindex-controller.js'; + import style from './BaseAccordion.css'; const CSS_TIMING_UNITS_RE = /^[0-9.]+(?[a-zA-Z]+)/g; @@ -48,6 +50,8 @@ export abstract class BaseAccordion extends LitElement { return target instanceof BaseAccordionPanel; } + #headerIndex = new RovingTabindexController(this); + /** * Sets and reflects the currently expanded accordion 0-based indexes. * Use commas to separate multiple indexes. @@ -78,6 +82,12 @@ export abstract class BaseAccordion extends LitElement { return this.#allPanels(); } + get activeHeader() { + const { headers } = this; + const index = headers.findIndex(header => header.matches(':focus,:focus-within')); + return headers.at(index); + } + protected expandedSets = new Set(); #logger = new Logger(this); @@ -103,7 +113,6 @@ export abstract class BaseAccordion extends LitElement { connectedCallback() { super.connectedCallback(); this.addEventListener('change', this.#onChange as EventListener); - this.#headerIndex.initItems(this.headers); this.#mo.observe(this, { childList: true }); this.#init(); } @@ -135,11 +144,19 @@ export abstract class BaseAccordion extends LitElement { */ async #init() { this.#initialized ||= !!await this.updateComplete; + this.#headerIndex.initItems(this.headers); // Event listener to the accordion header after the accordion has been initialized to add the roving tabindex this.addEventListener('focusin', this.#updateActiveHeader as EventListener); this.updateAccessibility(); } + #updateActiveHeader() { + const { activeHeader } = this; + if (activeHeader) { + this.#headerIndex.updateActiveItem(activeHeader); + } + } + #panelForHeader(header: BaseAccordionHeader) { const next = header.nextElementSibling; if (!BaseAccordion.isPanel(next)) { diff --git a/elements/pf-accordion/test/pf-accordion.spec.ts b/elements/pf-accordion/test/pf-accordion.spec.ts index f6e64ce85d..f4468b4317 100644 --- a/elements/pf-accordion/test/pf-accordion.spec.ts +++ b/elements/pf-accordion/test/pf-accordion.spec.ts @@ -374,12 +374,6 @@ describe('', function() { }); }); - describe('Tab', function() { - beforeEach(press('Tab')); - it('moves focus to the second header', function() { - expect(document.activeElement).to.equal(header2); - }); - }); describe('Shift+Tab', function() { beforeEach(press('Shift+Tab')); @@ -461,12 +455,6 @@ describe('', function() { }); }); - describe('Tab', function() { - beforeEach(press('Tab')); - it('moves focus to the last header', function() { - expect(document.activeElement).to.equal(header3); - }); - }); describe('ArrowDown', function() { beforeEach(press('ArrowDown')); @@ -537,12 +525,6 @@ describe('', function() { }); }); - describe('Shift+Tab', function() { - beforeEach(press('Shift+Tab')); - it('moves focus to the link in middle header', function() { - expect(document.activeElement).to.equal(header2); - }); - }); describe('ArrowDown', function() { beforeEach(press('ArrowDown')); @@ -605,9 +587,6 @@ describe('', function() { }); describe('Tab', function() { beforeEach(press('Tab')); - it('moves focus to the second header', function() { - expect(document.activeElement).to.equal(header2); - }); describe('Shift+Tab', function() { beforeEach(press('Shift+Tab')); it('keeps focus on the link in the first panel', function() { @@ -787,8 +766,8 @@ describe('', function() { }); }); - describe('Shift+Tab', function() { - beforeEach(press('Shift+Tab')); + describe('ArrowUp', function() { + beforeEach(press('ArrowUp')); it('moves focus to the first header', function() { expect(document.activeElement).to.equal(header1); }); From f38d2a7bb62bd30d7b558f4527867f7ac37894c7 Mon Sep 17 00:00:00 2001 From: Brian Ferry Date: Tue, 9 May 2023 10:19:30 -0400 Subject: [PATCH 09/14] fix(tabs): reverting code change made locally --- elements/pf-tabs/BaseTabs.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/elements/pf-tabs/BaseTabs.ts b/elements/pf-tabs/BaseTabs.ts index 88267bd0f6..ead02de97a 100644 --- a/elements/pf-tabs/BaseTabs.ts +++ b/elements/pf-tabs/BaseTabs.ts @@ -73,6 +73,8 @@ export abstract class BaseTabs extends LitElement { #activeIndex = 0; + id: string = this.id || getRandomId(this.localName); + /** * Tab activation * Tabs can be either [automatic](https://w3c.github.io/aria-practices/examples/tabs/tabs-automatic.html) activated @@ -137,7 +139,6 @@ export abstract class BaseTabs extends LitElement { override connectedCallback() { super.connectedCallback(); - this.id ||= getRandomId(this.localName); this.addEventListener('expand', this.#onTabExpand); BaseTabs.#instances.add(this); } From 93fce18aa78b5654a4e88c9a8aa85f40ece04d95 Mon Sep 17 00:00:00 2001 From: Benny Powers Date: Wed, 17 May 2023 15:30:04 +0300 Subject: [PATCH 10/14] docs: update changeset --- .changeset/empty-trainers-tickle.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.changeset/empty-trainers-tickle.md b/.changeset/empty-trainers-tickle.md index e997131227..bdf3bd45bf 100644 --- a/.changeset/empty-trainers-tickle.md +++ b/.changeset/empty-trainers-tickle.md @@ -1,6 +1,4 @@ --- "@patternfly/elements": patch --- - - -**pf-accordion**: Fixing keyboard navigation inside of nested accordions. \ No newline at end of file +``: fixd keyboard navigation inside of nested accordions From 23ddd2d9570c2fa56005c5f6bade412f60adeb17 Mon Sep 17 00:00:00 2001 From: Benny Powers Date: Wed, 17 May 2023 15:30:19 +0300 Subject: [PATCH 11/14] perf: defer computing composedPath() --- core/pfe-core/controllers/roving-tabindex-controller.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/core/pfe-core/controllers/roving-tabindex-controller.ts b/core/pfe-core/controllers/roving-tabindex-controller.ts index c576ba07d0..e4a2ca53f1 100644 --- a/core/pfe-core/controllers/roving-tabindex-controller.ts +++ b/core/pfe-core/controllers/roving-tabindex-controller.ts @@ -93,9 +93,12 @@ export class RovingTabindexController< * handles keyboard navigation */ #onKeydown = (event: KeyboardEvent) => { - const composed = event.composedPath(); - const targetIsFocusable = () => this.#focusableItems.filter(x=>composed.includes(x))?.length > 0; - if (event.ctrlKey || event.altKey || event.metaKey || this.#focusableItems.length < 1 || !targetIsFocusable()) { + if (event.ctrlKey || + event.altKey || + event.metaKey || + !this.#focusableItems.length || + !event.composedPath().some(x => + this.#focusableItems.includes(x as ItemType))) { return; } const item = this.activeItem; From a6ab38da74a127ae412178c5076aad8343e6553a Mon Sep 17 00:00:00 2001 From: Brian Ferry Date: Fri, 19 May 2023 10:42:20 -0400 Subject: [PATCH 12/14] test(accordion): adding test cases for rti navigation in nested, multiple accordions --- .../pf-accordion/test/pf-accordion.spec.ts | 258 ++++++++++++++++++ 1 file changed, 258 insertions(+) diff --git a/elements/pf-accordion/test/pf-accordion.spec.ts b/elements/pf-accordion/test/pf-accordion.spec.ts index f4468b4317..ef0a7098e3 100644 --- a/elements/pf-accordion/test/pf-accordion.spec.ts +++ b/elements/pf-accordion/test/pf-accordion.spec.ts @@ -374,6 +374,13 @@ describe('', function() { }); }); + describe('Tab', function() { + beforeEach(press('Tab')); + it('moves focus to the body', function() { + expect(document.activeElement).to.equal(document.body); + }); + }); + describe('Shift+Tab', function() { beforeEach(press('Shift+Tab')); @@ -455,6 +462,13 @@ describe('', function() { }); }); + describe('Tab', function() { + beforeEach(press('Tab')); + it('moves focus to the body', function() { + expect(document.activeElement).to.equal(document.body); + }); + }); + describe('ArrowDown', function() { beforeEach(press('ArrowDown')); @@ -525,6 +539,13 @@ describe('', function() { }); }); + describe('Shift+Tab', function() { + beforeEach(press('Shift+Tab')); + it('moves focus to the body', function() { + expect(document.activeElement).to.equal(document.body); + }); + }); + describe('ArrowDown', function() { beforeEach(press('ArrowDown')); @@ -587,6 +608,9 @@ describe('', function() { }); describe('Tab', function() { beforeEach(press('Tab')); + it('moves focus to the body', function() { + expect(document.activeElement).to.equal(header3); + }); describe('Shift+Tab', function() { beforeEach(press('Shift+Tab')); it('keeps focus on the link in the first panel', function() { @@ -766,12 +790,26 @@ describe('', function() { }); }); + describe('Shift+Tab', function() { + beforeEach(press('Shift+Tab')); + it('moves focus to the body', function() { + expect(document.activeElement).to.equal(document.body); + }); + }); + describe('ArrowUp', function() { beforeEach(press('ArrowUp')); it('moves focus to the first header', function() { expect(document.activeElement).to.equal(header1); }); }); + + describe('ArrowDown', function() { + beforeEach(press('ArrowDown')); + it('moves focus to the last header', function() { + expect(document.activeElement).to.equal(header3); + }); + }); }); describe('and focus is on the last header', function() { @@ -959,15 +997,18 @@ describe('', function() { describe('with nested pf-accordion', function() { let topLevelHeaderOne: PfAccordionHeader; let topLevelHeaderTwo: PfAccordionHeader; + let topLevelHeaderThree: PfAccordionHeader; let topLevelPanelOne: PfAccordionPanel; let topLevelPanelTwo: PfAccordionPanel; let nestedHeaderOne: PfAccordionHeader; let nestedHeaderTwo: PfAccordionHeader; + let nestedHeaderThree: PfAccordionHeader; let nestedPanelOne: PfAccordionPanel; let nestedPanelTwo: PfAccordionPanel; + let nestedPanelThree: PfAccordionPanel; beforeEach(async function() { element = await createFixture(html` @@ -986,6 +1027,8 @@ describe('', function() { + + @@ -995,15 +1038,18 @@ describe('', function() { `); topLevelHeaderOne = document.getElementById('header-1') as PfAccordionHeader; topLevelHeaderTwo = document.getElementById('header-2') as PfAccordionHeader; + topLevelHeaderThree = document.getElementById('header-3') as PfAccordionHeader; topLevelPanelOne = document.getElementById('panel-1') as PfAccordionPanel; topLevelPanelTwo = document.getElementById('panel-2') as PfAccordionPanel; nestedHeaderOne = document.getElementById('header-2-1') as PfAccordionHeader; nestedHeaderTwo = document.getElementById('header-2-2') as PfAccordionHeader; + nestedHeaderThree = document.getElementById('header-2-3') as PfAccordionHeader; nestedPanelOne = document.getElementById('panel-2-1') as PfAccordionPanel; nestedPanelTwo = document.getElementById('panel-2-2') as PfAccordionPanel; + nestedPanelThree = document.getElementById('panel-2-3') as PfAccordionPanel; await allUpdates(element); }); @@ -1056,5 +1102,217 @@ describe('', function() { }); }); }); + + describe('for assistive technology', function() { + describe('with all panels closed', function() { + it('applies hidden attribute to all panels', function() { + expect(topLevelPanelOne.hidden, 'panel-1').to.be.true; + expect(topLevelPanelTwo.hidden, 'panel-2').to.be.true; + expect(nestedPanelOne.hidden, 'panel-1-1').to.be.true; + expect(nestedPanelTwo.hidden, 'panel-2-2').to.be.true; + expect(nestedPanelThree.hidden, 'panel-2-3').to.be.true; + }); + }); + + describe('with all panels open', function() { + beforeEach(async function() { + for (const header of element.querySelectorAll('pf-accordion-header')) { + header.click(); + } + await nextFrame(); + }); + it('removes hidden attribute from all panels', function() { + expect(topLevelPanelOne.hidden, 'panel-1').to.be.false; + expect(topLevelPanelTwo.hidden, 'panel-2').to.be.false; + expect(nestedPanelOne.hidden, 'panel-1-1').to.be.false; + expect(nestedPanelTwo.hidden, 'panel-2-2').to.be.false; + expect(nestedPanelThree.hidden, 'panel-2-3').to.be.false; + }); + }); + + describe('when focus is on the first header of the parent accordion', function() { + beforeEach(async function() { + topLevelHeaderOne.focus(); + await nextFrame(); + }); + + describe('Space', function() { + beforeEach(press(' ')); + it('expands the first panel', function() { + expect(topLevelPanelOne.expanded).to.be.true; + expect(topLevelPanelTwo.expanded).to.be.false; + expect(nestedPanelOne.expanded).to.be.false; + expect(nestedPanelTwo.expanded).to.be.false; + }); + it('removes hidden attribute from the first panel', function() { + expect(topLevelPanelOne.hidden, 'panel-1').to.be.false; + expect(topLevelPanelTwo.hidden, 'panel-2').to.be.true; + expect(nestedPanelOne.hidden, 'panel-1-1').to.be.true; + expect(nestedPanelTwo.hidden, 'panel-2-2').to.be.true; + }); + }); + + describe('Tab', function() { + beforeEach(press('Tab')); + it('moves focus to the body', function() { + expect(document.activeElement).to.equal(document.body); + }); + }); + }); + + describe('when focus is on the last header of the parent accordion', function() { + beforeEach(async function() { + topLevelHeaderTwo.focus(); + await nextFrame(); + }); + + describe('Space', function() { + beforeEach(press(' ')); + it('expands the first panel', function() { + expect(topLevelPanelOne.expanded).to.be.false; + expect(topLevelPanelTwo.expanded).to.be.true; + expect(nestedPanelOne.expanded).to.be.false; + expect(nestedPanelTwo.expanded).to.be.false; + }); + it('removes hidden attribute from the first panel', function() { + expect(topLevelPanelOne.hidden, 'panel-2').to.be.true; + expect(topLevelPanelTwo.hidden, 'panel-1').to.be.false; + expect(nestedPanelOne.hidden, 'panel-1-1').to.be.true; + expect(nestedPanelTwo.hidden, 'panel-2-2').to.be.true; + }); + }); + + describe('Navigating from parent to child accordion', function() { + describe('Opening the panel containing the nested accordion and pressing TAB', function() { + beforeEach(press(' ')); + beforeEach(press('Tab')); + it('moves focus to the nested accordion header', function() { + expect(document.activeElement).to.equal(nestedHeaderOne); + }); + + describe('ArrowUp', function() { + beforeEach(press('ArrowUp')); + it('moves focus to the last header', function() { + expect(document.activeElement).to.equal(nestedHeaderThree); + }); + }); + + describe('ArrowLeft', function() { + beforeEach(press('ArrowLeft')); + it('moves focus to the last header', function() { + expect(document.activeElement).to.equal(nestedHeaderThree); + }); + }); + + describe('ArrowDown', function() { + beforeEach(press('ArrowDown')); + it('moves focus to the second header', function() { + expect(document.activeElement).to.equal(nestedHeaderTwo); + }); + }); + + describe('ArrowRight', function() { + beforeEach(press('ArrowRight')); + it('moves focus to the second header', function() { + expect(document.activeElement).to.equal(nestedHeaderTwo); + }); + }); + + describe('Tab', function() { + beforeEach(press('Tab')); + it('should move focus back to the parent accordion', function() { + expect(document.activeElement).to.equal(topLevelHeaderThree); + }); + }); + }); + }); + }); + }); + }); + + describe('with multiple pf-accordion', function() { + let multipleAccordionElements: HTMLElement; + + let accordionOneHeaderOne: PfAccordionHeader; + let accordionOnePanelOne: PfAccordionPanel; + + let accordionTwoHeaderOne: PfAccordionHeader; + let accordionTwoPanelOne: PfAccordionPanel; + + beforeEach(async function() { + multipleAccordionElements = await createFixture(html` +
+ + + + + + + + +
+ `); + accordionOneHeaderOne = document.getElementById('header-1-1') as PfAccordionHeader; + + accordionOnePanelOne = document.getElementById('panel-1-1') as PfAccordionPanel; + + accordionTwoHeaderOne = document.getElementById('header-2-1') as PfAccordionHeader; + + accordionTwoPanelOne = document.getElementById('panel-2-1') as PfAccordionPanel; + }); + + describe('for assistive technology', function() { + describe('with all panels closed', function() { + it('applies hidden attribute to all panels', function() { + expect(accordionOnePanelOne.hidden, 'panel-1-1').to.be.true; + expect(accordionTwoPanelOne.hidden, 'panel-2-1').to.be.true; + }); + }); + + describe('with all panels open', function() { + beforeEach(async function() { + for (const header of multipleAccordionElements.querySelectorAll('pf-accordion-header')) { + header.click(); + } + await nextFrame(); + }); + it('removes hidden attribute from all panels', function() { + expect(accordionOnePanelOne.hidden, 'panel-1-1').to.be.false; + expect(accordionTwoPanelOne.hidden, 'panel-2-1').to.be.false; + }); + }); + + describe('when focus is on the first header of the first accordion', function() { + beforeEach(async function() { + accordionOneHeaderOne.focus(); + await nextFrame(); + }); + + describe('Space', function() { + beforeEach(press(' ')); + it('expands the first panel', function() { + expect(accordionOnePanelOne.expanded).to.be.true; + expect(accordionTwoPanelOne.expanded).to.be.false; + }); + it('removes hidden attribute from the first panel', function() { + expect(accordionOnePanelOne.hidden, 'panel-1-1').to.be.false; + expect(accordionTwoPanelOne.hidden, 'panel-1-1').to.be.true; + }); + }); + + describe('Tab', function() { + beforeEach(press('Tab')); + it('moves focus to the second accordion', function() { + expect(document.activeElement).to.equal(accordionTwoHeaderOne); + }); + describe('Shift+Tab', function() { + beforeEach(press('Shift+Tab')); + it('moves focus back to the first accordion', function() { + expect(document.activeElement).to.equal(accordionOneHeaderOne); + }); + }); + }); + }); + }); }); }); From b339a94eab0720613694c25c1a694c0897bcf72b Mon Sep 17 00:00:00 2001 From: Brian Ferry Date: Mon, 22 May 2023 13:53:24 -0400 Subject: [PATCH 13/14] fix(accordion): activeHeader a private accessor --- elements/pf-accordion/BaseAccordion.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/elements/pf-accordion/BaseAccordion.ts b/elements/pf-accordion/BaseAccordion.ts index 10906c7ccf..bfcfc93ff0 100644 --- a/elements/pf-accordion/BaseAccordion.ts +++ b/elements/pf-accordion/BaseAccordion.ts @@ -82,7 +82,7 @@ export abstract class BaseAccordion extends LitElement { return this.#allPanels(); } - get activeHeader() { + get #activeHeader() { const { headers } = this; const index = headers.findIndex(header => header.matches(':focus,:focus-within')); return headers.at(index); @@ -151,9 +151,8 @@ export abstract class BaseAccordion extends LitElement { } #updateActiveHeader() { - const { activeHeader } = this; - if (activeHeader) { - this.#headerIndex.updateActiveItem(activeHeader); + if (this.#activeHeader) { + this.#headerIndex.updateActiveItem(this.#activeHeader); } } From 130cb5c24978a96297c64fabcf21373b8e937d1f Mon Sep 17 00:00:00 2001 From: Benny Powers Date: Sun, 28 May 2023 12:56:52 +0300 Subject: [PATCH 14/14] docs: update .changeset/empty-trainers-tickle.md --- .changeset/empty-trainers-tickle.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/empty-trainers-tickle.md b/.changeset/empty-trainers-tickle.md index bdf3bd45bf..9f6b32a0f6 100644 --- a/.changeset/empty-trainers-tickle.md +++ b/.changeset/empty-trainers-tickle.md @@ -1,4 +1,4 @@ --- "@patternfly/elements": patch --- -``: fixd keyboard navigation inside of nested accordions +``: fixed keyboard navigation inside of nested accordions