diff --git a/packages/components-dev/accordion/module.ts b/packages/components-dev/accordion/module.ts index fa523dce14..5158d22cf0 100644 --- a/packages/components-dev/accordion/module.ts +++ b/packages/components-dev/accordion/module.ts @@ -1,10 +1,37 @@ import { ChangeDetectionStrategy, Component, ViewEncapsulation } from '@angular/core'; import { KbqAccordionModule } from '@koobiq/components/accordion'; import { KbqIconModule } from '@koobiq/components/icon'; +import { AccordionExamplesModule } from 'packages/docs-examples/components/accordion'; + +@Component({ + selector: 'dev-examples', + imports: [AccordionExamplesModule], + template: ` + +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ + `, + changeDetection: ChangeDetectionStrategy.OnPush +}) +export class DevDocsExamples {} @Component({ selector: 'dev-app', - imports: [KbqAccordionModule, KbqIconModule], + imports: [KbqAccordionModule, KbqIconModule, DevDocsExamples], templateUrl: './template.html', styleUrls: ['./styles.scss'], changeDetection: ChangeDetectionStrategy.OnPush, diff --git a/packages/components-dev/accordion/template.html b/packages/components-dev/accordion/template.html index eabef6bebd..d1091eb010 100644 --- a/packages/components-dev/accordion/template.html +++ b/packages/components-dev/accordion/template.html @@ -1,3 +1,7 @@ + + +
+ diff --git a/packages/components/accordion/_accordion-theme.scss b/packages/components/accordion/_accordion-theme.scss index 135b5f0a09..deb41fc97a 100644 --- a/packages/components/accordion/_accordion-theme.scss +++ b/packages/components/accordion/_accordion-theme.scss @@ -1,8 +1,12 @@ @use '../core/styles/common/tokens' as *; @mixin kbq-accordion-theme() { + // Only the trigger owns the item's focus ring. `:focus-within` would also fire for header + // actions and for controls inside the expanded content, ringing a section the user is merely + // typing in. Both shapes are matched because `` is optional. .kbq-accordion.cdk-keyboard-focused { - & .kbq-accordion-item:focus-within { + & .kbq-accordion-item:has(> .kbq-accordion-trigger:focus), + & .kbq-accordion-item:has(> .kbq-accordion-header .kbq-accordion-trigger:focus) { border-color: var(--kbq-accordion-item-states-focus-border-color); } } diff --git a/packages/components/accordion/accordion-content.directive.ts b/packages/components/accordion/accordion-content.directive.ts index d04c16ab6e..6bc8a0b900 100644 --- a/packages/components/accordion/accordion-content.directive.ts +++ b/packages/components/accordion/accordion-content.directive.ts @@ -17,6 +17,11 @@ import { KbqAccordionItem } from './accordion-item'; '[attr.id]': 'contentId', '[attr.role]': '"region"', '[attr.hidden]': 'hidden() ? "" : null', + // `hidden` alone does not take the collapsed content out of the tab order: the host keeps an + // explicit `display: block` so the height can animate, which overrides `[hidden]`'s + // `display: none`. Without `inert`, every control inside a closed section stays focusable in + // a zero-height, clipped box. + '[attr.inert]': 'hidden() ? "" : null', '[attr.aria-labelledby]': 'triggerId', '[attr.data-state]': 'item.dataState', diff --git a/packages/components/accordion/accordion-header.ts b/packages/components/accordion/accordion-header.ts index 51a52bf844..7ff37f8530 100644 --- a/packages/components/accordion/accordion-header.ts +++ b/packages/components/accordion/accordion-header.ts @@ -8,6 +8,7 @@ import { KbqAccordionItem } from './accordion-item'; class: 'kbq-accordion-header', '[attr.role]': '"heading"', '[attr.aria-level]': 'accordion.level()', + '[attr.aria-labelledby]': 'labelledBy', '[attr.data-state]': 'item.dataState', '[attr.data-disabled]': 'item.disabled', '[attr.data-orientation]': 'item.orientation' @@ -18,4 +19,18 @@ export class KbqAccordionHeader { protected readonly item = inject(KbqAccordionItem); /** @docs-private */ protected readonly accordion = inject(KbqAccordion); + + /** + * Names the heading after the trigger alone. + * + * A `role="heading"` takes its name from its content, so the header actions sitting beside the + * trigger would otherwise append their own labels to it — a section titled "Profile" would be + * announced as "Profile Run More" when navigating by heading. Pointing at the trigger yields the + * announcement the WAI-ARIA APG gets by keeping the button the only element in the heading, + * which this component cannot do because the header is also the row that lays the actions out. + * @docs-private + */ + protected get labelledBy(): string | null { + return this.item.trigger()?.triggerId ?? null; + } } diff --git a/packages/components/accordion/accordion-item.ts b/packages/components/accordion/accordion-item.ts index 3414bce3ab..92a4ac081c 100644 --- a/packages/components/accordion/accordion-item.ts +++ b/packages/components/accordion/accordion-item.ts @@ -33,8 +33,11 @@ export type KbqAccordionItemState = 'open' | 'closed'; } }) export class KbqAccordionItem implements OnDestroy { - /** @docs-private */ - protected readonly accordion = inject(KbqAccordion); + /** + * The accordion this item belongs to — always the nearest one, because it is injected. + * The accordion reads it back to tell its own items apart from a nested accordion's. + */ + readonly accordion = inject(KbqAccordion); /** @docs-private */ protected readonly changeDetectorRef = inject(ChangeDetectorRef); /** @docs-private */ diff --git a/packages/components/accordion/accordion-tokens.scss b/packages/components/accordion/accordion-tokens.scss index c1acdff88b..3b3185ccec 100644 --- a/packages/components/accordion/accordion-tokens.scss +++ b/packages/components/accordion/accordion-tokens.scss @@ -5,6 +5,8 @@ --kbq-accordion-size-item-header-variant-hug-padding: var(--kbq-size-xs) var(--kbq-size-s) var(--kbq-size-xs) var(--kbq-size-m); --kbq-accordion-size-item-content-padding: 0px var(--kbq-size-m) var(--kbq-size-s) var(--kbq-size-m); + --kbq-accordion-size-item-header-actions-gap: var(--kbq-size-3xs); + --kbq-accordion-size-item-header-actions-padding-right: var(--kbq-size-s); /* THEME TOKENS */ --kbq-accordion-item-default-background: transparent; --kbq-accordion-item-default-text-color: var(--kbq-foreground-contrast); diff --git a/packages/components/accordion/accordion-trigger.directive.ts b/packages/components/accordion/accordion-trigger.directive.ts index c7a992b944..5ffdf0a021 100644 --- a/packages/components/accordion/accordion-trigger.directive.ts +++ b/packages/components/accordion/accordion-trigger.directive.ts @@ -15,7 +15,8 @@ import { KbqAccordionItem } from './accordion-item'; '[attr.data-disabled]': 'item.disabled', '[attr.data-orientation]': 'item.orientation', '(click)': 'onClick()', - '(focus)': 'onFocus()' + '(focus)': 'onFocus()', + '(keydown)': 'onKeydown($event)' } }) export class KbqAccordionTriggerDirective { @@ -54,6 +55,24 @@ export class KbqAccordionTriggerDirective { this.accordion.setActiveItem(this.item); } + /** + * Routes keyboard interaction to the accordion, but only while the trigger itself is focused. + * + * A `keydown` targets the focused element, so header actions placed next to the trigger and + * controls inside the expanded content keep their own keys — the accordion no longer toggles the + * section on Enter/Space nor moves focus to another header on the arrow keys. The `target` check + * additionally covers focusable content mistakenly nested inside the trigger. + */ + onKeydown(event: KeyboardEvent): void { + if (event.target !== this.nativeElement) return; + + // The key manager can lag behind when focus was moved programmatically rather than by the + // user, in which case `onFocus` never ran. + this.accordion.setActiveItem(this.item); + + this.accordion.keydownHandler(event); + } + /** @docs-private */ focus() { this.nativeElement.focus(); diff --git a/packages/components/accordion/accordion-trigger.scss b/packages/components/accordion/accordion-trigger.scss index 31695df979..0b8a150fc9 100644 --- a/packages/components/accordion/accordion-trigger.scss +++ b/packages/components/accordion/accordion-trigger.scss @@ -23,6 +23,8 @@ margin-right: var(--kbq-size-s); } + // @deprecated Actions nested inside the trigger are focusable elements inside a `role="button"`. + // Put them in a `.kbq-accordion-header__actions` sibling of the trigger instead. & .kbq-accordion-trigger__action { padding: var(--kbq-size-xxs) var(--kbq-size-xxs) var(--kbq-size-3xs); margin-bottom: calc(-1 * #{var(--kbq-size-3xs)}); @@ -61,4 +63,8 @@ button.kbq-accordion-trigger { flex: 1; + + // A flex item floors at its min-content width by default, which would push the header actions + // past the item's right edge instead of letting a long label give way. + min-width: 0; } diff --git a/packages/components/accordion/accordion.en.md b/packages/components/accordion/accordion.en.md index 3518e8f406..8268b8a722 100644 --- a/packages/components/accordion/accordion.en.md +++ b/packages/components/accordion/accordion.en.md @@ -38,6 +38,12 @@ This area can contain any type of content. +##### Interactive Elements + +Buttons, dropdown menus and form controls can be placed in the section header next to the trigger, as well as inside the content area. Place them **next to** the trigger, never inside it: the trigger is a `role="button"`, and nesting focusable elements in it breaks accessibility. Enter and Space toggle the section only while the trigger itself is focused. + + + ### Usage Examples #### Inside a Section diff --git a/packages/components/accordion/accordion.ru.md b/packages/components/accordion/accordion.ru.md index 3d07d11881..851727d4f1 100644 --- a/packages/components/accordion/accordion.ru.md +++ b/packages/components/accordion/accordion.ru.md @@ -38,6 +38,12 @@ +##### Интерактивные элементы + +Кнопки, выпадающие меню и элементы формы можно размещать в шапке секции рядом с триггером, а также внутри области контента. Размещайте их **рядом** с триггером, но не внутри него: триггер имеет роль `button`, и вложение в него фокусируемых элементов нарушает доступность. Enter и Space раскрывают и сворачивают секцию только когда фокус находится на самом триггере. + + + ### Примеры использования #### Внутри секции diff --git a/packages/components/accordion/accordion.scss b/packages/components/accordion/accordion.scss index 40c67f4d57..7f55732e7a 100644 --- a/packages/components/accordion/accordion.scss +++ b/packages/components/accordion/accordion.scss @@ -17,6 +17,27 @@ display: flex; height: var(--kbq-accordion-size-item-header-height); + // The height above is fixed, so a trigger label long enough to wrap would paint outside the + // header and over the item's border. Clipping keeps it inside; the trigger is a flex container, + // so `text-overflow` cannot ellipsize its text — wrap the label in an element of your own if you + // need that. + overflow: hidden; + + // Interactive controls live NEXT TO the trigger, never inside it: the trigger is `role="button"` + // and nesting focusable elements in it is an accessibility violation. The trigger's `flex: 1` + // keeps the rest of the header clickable, so the actions sit flush right; their padding matches + // the trigger's own right padding, which is `--kbq-size-s` in every variant. + & > .kbq-accordion-header__actions { + display: flex; + align-items: center; + gap: var(--kbq-accordion-size-item-header-actions-gap); + + // Icon buttons have a fixed size and nothing to reflow, so the trigger — not the actions — + // is what gives way when the header runs out of room. + flex-shrink: 0; + + padding-right: var(--kbq-accordion-size-item-header-actions-padding-right); + } } .kbq-accordion-content { diff --git a/packages/components/accordion/accordion.spec.ts b/packages/components/accordion/accordion.spec.ts index 901223be6d..c7fdb5005d 100644 --- a/packages/components/accordion/accordion.spec.ts +++ b/packages/components/accordion/accordion.spec.ts @@ -67,7 +67,10 @@ describe('KbqAccordion', () => { AccordionValueMultiple, AccordionMissingContent, AccordionLevel, - AccordionStateSaving + AccordionStateSaving, + AccordionInteractiveContent, + AccordionNestedInTrigger, + AccordionNested ] }).compileComponents(); }); @@ -477,7 +480,7 @@ describe('KbqAccordion', () => { accordion.setActiveItem(items[0].componentInstance as KbqAccordionItem); - dispatchKeyboardEvent(accordionEl.nativeElement, 'keydown', ENTER); + dispatchKeyboardEvent(triggers[0].nativeElement, 'keydown', ENTER); fixture.detectChanges(); expect(items[0].nativeElement.getAttribute('data-state')).toBe('closed'); @@ -489,12 +492,13 @@ describe('KbqAccordion', () => { const accordionEl = fixture.debugElement.query(By.directive(KbqAccordion)); const items = fixture.debugElement.queryAll(By.directive(KbqAccordionItem)); + const triggers = fixture.debugElement.queryAll(By.directive(KbqAccordionTrigger)); const accordion = accordionEl.componentInstance as KbqAccordion; accordion.setActiveItem(items[0].componentInstance as KbqAccordionItem); - dispatchKeyboardEvent(accordionEl.nativeElement, 'keydown', SPACE); + dispatchKeyboardEvent(triggers[0].nativeElement, 'keydown', SPACE); fixture.detectChanges(); expect(items[0].nativeElement.getAttribute('data-state')).toBe('open'); @@ -513,7 +517,7 @@ describe('KbqAccordion', () => { accordion.setActiveItem(items[0].injector.get(KbqAccordionItem)); expect(document.activeElement).toBe(triggers[0].nativeElement); - dispatchKeyboardEvent(accordionEl.nativeElement, 'keydown', DOWN_ARROW); + dispatchKeyboardEvent(triggers[0].nativeElement, 'keydown', DOWN_ARROW); fixture.detectChanges(); expect(document.activeElement).toBe(triggers[1].nativeElement); @@ -532,7 +536,7 @@ describe('KbqAccordion', () => { accordion.setActiveItem(items[1].injector.get(KbqAccordionItem)); expect(document.activeElement).toBe(triggers[1].nativeElement); - dispatchKeyboardEvent(accordionEl.nativeElement, 'keydown', UP_ARROW); + dispatchKeyboardEvent(triggers[1].nativeElement, 'keydown', UP_ARROW); fixture.detectChanges(); expect(document.activeElement).toBe(triggers[0].nativeElement); @@ -550,11 +554,12 @@ describe('KbqAccordion', () => { accordion.setActiveItem(items[0].injector.get(KbqAccordionItem)); - dispatchKeyboardEvent(accordionEl.nativeElement, 'keydown', END); + dispatchKeyboardEvent(triggers[0].nativeElement, 'keydown', END); fixture.detectChanges(); expect(document.activeElement).toBe(triggers[triggers.length - 1].nativeElement); - dispatchKeyboardEvent(accordionEl.nativeElement, 'keydown', HOME); + // Focus is on the last trigger now, so that is where the next key really comes from. + dispatchKeyboardEvent(triggers[triggers.length - 1].nativeElement, 'keydown', HOME); fixture.detectChanges(); expect(document.activeElement).toBe(triggers[0].nativeElement); }); @@ -565,11 +570,12 @@ describe('KbqAccordion', () => { const accordionEl = fixture.debugElement.query(By.directive(KbqAccordion)); const items = fixture.debugElement.queryAll(By.directive(KbqAccordionItem)); + const triggers = fixture.debugElement.queryAll(By.directive(KbqAccordionTrigger)); const accordion = accordionEl.componentInstance as KbqAccordion; accordion.setActiveItem(items[0].injector.get(KbqAccordionItem)); - const event = dispatchKeyboardEvent(accordionEl.nativeElement, 'keydown', TAB); + const event = dispatchKeyboardEvent(triggers[0].nativeElement, 'keydown', TAB); fixture.detectChanges(); @@ -584,10 +590,11 @@ describe('KbqAccordion', () => { const accordionEl = fixture.debugElement.query(By.directive(KbqAccordion)); const accordion = accordionEl.componentInstance as KbqAccordion; const items = fixture.debugElement.queryAll(By.directive(KbqAccordionItem)); + const triggers = fixture.debugElement.queryAll(By.directive(KbqAccordionTrigger)); accordion.setActiveItem(items[0].injector.get(KbqAccordionItem)); - dispatchKeyboardEvent(accordionEl.nativeElement, 'keydown', ENTER); + dispatchKeyboardEvent(triggers[0].nativeElement, 'keydown', ENTER); fixture.detectChanges(); expect(items[0].nativeElement.getAttribute('data-state')).toBe('closed'); @@ -613,11 +620,11 @@ describe('KbqAccordion', () => { accordion.setActiveItem(items[0].injector.get(KbqAccordionItem)); - dispatchKeyboardEvent(accordionEl.nativeElement, 'keydown', RIGHT_ARROW); + dispatchKeyboardEvent(triggers[0].nativeElement, 'keydown', RIGHT_ARROW); fixture.detectChanges(); expect(document.activeElement).toBe(triggers[1].nativeElement); - dispatchKeyboardEvent(accordionEl.nativeElement, 'keydown', LEFT_ARROW); + dispatchKeyboardEvent(triggers[1].nativeElement, 'keydown', LEFT_ARROW); fixture.detectChanges(); expect(document.activeElement).toBe(triggers[0].nativeElement); }); @@ -639,12 +646,253 @@ describe('KbqAccordion', () => { accordion.setActiveItem(items[1].injector.get(KbqAccordionItem)); // In RTL, Right arrow moves to the previous item. - dispatchKeyboardEvent(accordionEl.nativeElement, 'keydown', RIGHT_ARROW); + dispatchKeyboardEvent(triggers[1].nativeElement, 'keydown', RIGHT_ARROW); fixture.detectChanges(); expect(document.activeElement).toBe(triggers[0].nativeElement); }); }); + describe('interactive content', () => { + // Attaching to the document is required for `document.activeElement` focus assertions to work. + afterEach(() => { + if (fixture?.nativeElement?.parentNode === document.body) { + document.body.removeChild(fixture.nativeElement); + } + }); + + /** Creates the interactive fixture, attaches it to the document and returns its parts. */ + const createInteractiveFixture = () => { + fixture = TestBed.createComponent(AccordionInteractiveContent); + fixture.detectChanges(); + document.body.appendChild(fixture.nativeElement); + + return { + accordion: fixture.debugElement.query(By.directive(KbqAccordion)), + items: fixture.debugElement.queryAll(By.directive(KbqAccordionItem)), + triggers: fixture.debugElement.queryAll(By.directive(KbqAccordionTrigger)), + headerAction: fixture.nativeElement.querySelector('#header-action') as HTMLButtonElement, + secondaryHeaderAction: fixture.nativeElement.querySelector( + '#header-action-secondary' + ) as HTMLButtonElement, + disabledItemAction: fixture.nativeElement.querySelector('#disabled-item-action') as HTMLButtonElement, + contentInput: fixture.nativeElement.querySelector('#content-input') as HTMLInputElement + }; + }; + + it('ENTER on a header action should not toggle the item', () => { + const { items, headerAction } = createInteractiveFixture(); + + const event = dispatchKeyboardEvent(headerAction, 'keydown', ENTER); + + fixture.detectChanges(); + + expect(items[0].nativeElement.getAttribute('data-state')).toBe('closed'); + expect(event.defaultPrevented).toBe(false); + }); + + it('SPACE inside the expanded content should not collapse the item', () => { + const { items, triggers, contentInput } = createInteractiveFixture(); + + triggers[0].nativeElement.click(); + fixture.detectChanges(); + expect(items[0].nativeElement.getAttribute('data-state')).toBe('open'); + + const event = dispatchKeyboardEvent(contentInput, 'keydown', SPACE); + + fixture.detectChanges(); + + expect(items[0].nativeElement.getAttribute('data-state')).toBe('open'); + expect(event.defaultPrevented).toBe(false); + }); + + it('arrow keys inside the content should not move focus between headers', () => { + const { triggers, contentInput } = createInteractiveFixture(); + + triggers[0].nativeElement.click(); + fixture.detectChanges(); + + contentInput.focus(); + expect(document.activeElement).toBe(contentInput); + + dispatchKeyboardEvent(contentInput, 'keydown', DOWN_ARROW); + fixture.detectChanges(); + + expect(document.activeElement).toBe(contentInput); + }); + + it('HOME inside the content should not move focus to the first header', () => { + const { triggers, contentInput } = createInteractiveFixture(); + + triggers[0].nativeElement.click(); + fixture.detectChanges(); + + contentInput.focus(); + + dispatchKeyboardEvent(contentInput, 'keydown', HOME); + fixture.detectChanges(); + + expect(document.activeElement).toBe(contentInput); + }); + + it('a key pressed inside the content should not activate the first item', () => { + const { items, contentInput } = createInteractiveFixture(); + + // No `setActiveItem` beforehand: the removed `setFirstItemActive()` used to focus the + // first trigger and toggle it on the very first key pressed anywhere inside the accordion. + contentInput.focus(); + + dispatchKeyboardEvent(contentInput, 'keydown', ENTER); + fixture.detectChanges(); + + expect(document.activeElement).toBe(contentInput); + expect(items[0].nativeElement.getAttribute('data-state')).toBe('closed'); + expect(items[1].nativeElement.getAttribute('data-state')).toBe('closed'); + }); + + it('the accordion host should not handle keys itself', () => { + const { accordion, items } = createInteractiveFixture(); + + (accordion.componentInstance as KbqAccordion).setActiveItem(items[0].injector.get(KbqAccordionItem)); + + const event = dispatchKeyboardEvent(accordion.nativeElement, 'keydown', ENTER); + + fixture.detectChanges(); + + expect(items[0].nativeElement.getAttribute('data-state')).toBe('closed'); + expect(event.defaultPrevented).toBe(false); + }); + + it('ENTER on a focusable element nested inside the trigger should not toggle the item', () => { + fixture = TestBed.createComponent(AccordionNestedInTrigger); + fixture.detectChanges(); + + const item = fixture.debugElement.query(By.directive(KbqAccordionItem)); + const nested = fixture.nativeElement.querySelector('#nested-in-trigger') as HTMLElement; + + const event = dispatchKeyboardEvent(nested, 'keydown', ENTER); + + fixture.detectChanges(); + + expect(item.nativeElement.getAttribute('data-state')).toBe('closed'); + expect(event.defaultPrevented).toBe(false); + }); + + it('ENTER on the second header action should not toggle the item either', () => { + const { items, secondaryHeaderAction } = createInteractiveFixture(); + + const event = dispatchKeyboardEvent(secondaryHeaderAction, 'keydown', ENTER); + + fixture.detectChanges(); + + expect(items[0].nativeElement.getAttribute('data-state')).toBe('closed'); + expect(event.defaultPrevented).toBe(false); + }); + + it('header actions should be reachable and stay interactive on a disabled item', () => { + const { items, disabledItemAction } = createInteractiveFixture(); + const clicked = jest.fn(); + + expect(items[2].nativeElement.getAttribute('data-disabled')).toBe('true'); + + // Disabling an item only bars toggling it; the actions beside its trigger are ordinary + // buttons the consumer owns, so they keep their focus and their clicks. + disabledItemAction.addEventListener('click', clicked); + disabledItemAction.focus(); + disabledItemAction.click(); + + expect(document.activeElement).toBe(disabledItemAction); + expect(clicked).toHaveBeenCalled(); + }); + + it('ENTER on a disabled item header action should not toggle the item', () => { + const { items, disabledItemAction } = createInteractiveFixture(); + + const event = dispatchKeyboardEvent(disabledItemAction, 'keydown', ENTER); + + fixture.detectChanges(); + + expect(items[2].nativeElement.getAttribute('data-state')).toBe('closed'); + expect(event.defaultPrevented).toBe(false); + }); + + it('should make the content of a collapsed item inert', () => { + const { items, triggers } = createInteractiveFixture(); + const content = items[0].nativeElement.querySelector('kbq-accordion-content') as HTMLElement; + + // `hidden` cannot do this on its own: the host keeps an explicit `display: block` so the + // height can animate, which overrides `[hidden]`'s `display: none` and leaves every + // control inside a closed section focusable. + expect(content.hasAttribute('inert')).toBe(true); + + triggers[0].nativeElement.click(); + fixture.detectChanges(); + + expect(items[0].nativeElement.getAttribute('data-state')).toBe('open'); + expect(content.hasAttribute('inert')).toBe(false); + + triggers[0].nativeElement.click(); + fixture.detectChanges(); + + expect(content.hasAttribute('inert')).toBe(true); + }); + }); + + describe('nested accordion', () => { + /** Creates the nested fixture, attaches it to the document and returns its parts. */ + const createNestedFixture = () => { + fixture = TestBed.createComponent(AccordionNested); + fixture.detectChanges(); + document.body.appendChild(fixture.nativeElement); + + const accordions = fixture.debugElement.queryAll(By.directive(KbqAccordion)); + + return { + outer: accordions[0].componentInstance as KbqAccordion, + inner: accordions[1].componentInstance as KbqAccordion, + triggers: fixture.debugElement.queryAll(By.directive(KbqAccordionTrigger)), + host: fixture.componentInstance as AccordionNested + }; + }; + + afterEach(() => { + if (fixture?.nativeElement?.parentNode === document.body) { + document.body.removeChild(fixture.nativeElement); + } + }); + + it('should not claim the items of a nested accordion', () => { + const { outer, inner } = createNestedFixture(); + + expect(outer.items().map((item) => item.value())).toEqual(['outer-1', 'outer-2']); + expect(inner.items().map((item) => item.value())).toEqual(['inner-1', 'inner-2']); + }); + + it('arrow keys should move between the outer headers, skipping the nested ones', () => { + const { triggers } = createNestedFixture(); + // Document order is outer-1, inner-1, inner-2, outer-2. + const [outerTrigger1, , , outerTrigger2] = triggers.map((trigger) => trigger.nativeElement); + + outerTrigger1.focus(); + + dispatchKeyboardEvent(outerTrigger1, 'keydown', DOWN_ARROW); + fixture.detectChanges(); + + expect(document.activeElement).toBe(outerTrigger2); + }); + + it('toggling a nested item should not emit the outer valueChange', () => { + const { host, triggers } = createNestedFixture(); + + host.outerValueChanges.length = 0; + + // Index 1 is the first trigger of the nested accordion. + triggers[1].nativeElement.click(); + fixture.detectChanges(); + + expect(host.outerValueChanges).toEqual([]); + }); + }); + describe('orientation', () => { it('default orientation should be vertical', () => { fixture = TestBed.createComponent(AccordionOrientation); @@ -888,6 +1136,37 @@ describe('KbqAccordion', () => { accordionHeaderDebugElement = fixture.debugElement.query(By.directive(KbqAccordionHeader)); expect(accordionHeaderDebugElement.nativeElement.getAttribute('role')).toBe('heading'); }); + + it('should name the heading after the trigger', () => { + fixture = TestBed.createComponent(TestApp); + fixture.detectChanges(); + + accordionHeaderDebugElement = fixture.debugElement.query(By.directive(KbqAccordionHeader)); + accordionTriggerDebugElement = fixture.debugElement.query(By.directive(KbqAccordionTrigger)); + + expect(accordionHeaderDebugElement.nativeElement.getAttribute('aria-labelledby')).toBe( + accordionTriggerDebugElement.nativeElement.getAttribute('id') + ); + }); + + it('should keep header action labels out of the heading name', () => { + fixture = TestBed.createComponent(AccordionInteractiveContent); + fixture.detectChanges(); + + const header = fixture.debugElement.query(By.directive(KbqAccordionHeader)).nativeElement; + const labelSource = fixture.nativeElement.querySelector( + `#${header.getAttribute('aria-labelledby')}` + ) as HTMLElement; + + const actionLabels = Array.from( + header.querySelectorAll('.kbq-accordion-header__actions [aria-label]') + ).map((action) => (action as HTMLElement).getAttribute('aria-label')); + + // The labelled actions really do sit inside the heading — without the explicit + // reference, name-from-content would append both of them to the section title. + expect(actionLabels).toEqual(['Run Trigger 1', 'More for Trigger 1']); + expect(labelSource.textContent?.trim()).toBe('Trigger 1'); + }); }); describe('aria-disabled on trigger', () => { @@ -1249,6 +1528,58 @@ describe('KbqAccordion', () => { expect(await axe(fixture.nativeElement)).toHaveNoViolations(); }); + + // Pins the `nested-interactive` fix: actions must be siblings of the trigger, never inside + // it, because the trigger is a `role="button"`. The fixture also holds a disabled item with + // its own action, so the disabled styling is covered in both states. + it('has no axe violations with collapsed header actions', async () => { + fixture = TestBed.createComponent(AccordionInteractiveContent); + fixture.detectChanges(); + document.body.appendChild(fixture.nativeElement); + + expect(await axe(fixture.nativeElement)).toHaveNoViolations(); + }); + + it('has no axe violations with header actions and expanded content controls', async () => { + fixture = TestBed.createComponent(AccordionInteractiveContent); + fixture.detectChanges(); + document.body.appendChild(fixture.nativeElement); + + fixture.debugElement.query(By.directive(KbqAccordionTrigger)).nativeElement.click(); + fixture.detectChanges(); + + expect(await axe(fixture.nativeElement)).toHaveNoViolations(); + }); + + it('has no axe violations with header actions in RTL', async () => { + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + imports: [KbqAccordionModule, AccordionInteractiveContent], + providers: [ + { provide: Directionality, useValue: { value: 'rtl', change: EMPTY } } + ] + }); + + fixture = TestBed.createComponent(AccordionInteractiveContent); + fixture.detectChanges(); + document.body.appendChild(fixture.nativeElement); + + fixture.debugElement.query(By.directive(KbqAccordionTrigger)).nativeElement.click(); + fixture.detectChanges(); + + expect(await axe(fixture.nativeElement)).toHaveNoViolations(); + }); + + it('has no axe violations for a nested accordion', async () => { + fixture = TestBed.createComponent(AccordionNested); + fixture.detectChanges(); + document.body.appendChild(fixture.nativeElement); + + fixture.debugElement.query(By.directive(KbqAccordionTrigger)).nativeElement.click(); + fixture.detectChanges(); + + expect(await axe(fixture.nativeElement)).toHaveNoViolations(); + }); }); }); @@ -1621,3 +1952,100 @@ class AccordionLevel { ` }) class AccordionStateSaving {} + +// The action buttons are deliberately icon-only (empty, named solely by `aria-label`), matching what +// the docs ship: a button with its own visible text would keep the axe checks passing even if the +// pattern stopped exposing an accessible name at all. +@Component({ + selector: 'accordion-interactive-content', + imports: [KbqAccordionModule], + template: ` + + + + +
+ + +
+ + + + + + + + + + Content 2 + + + + +
+ +
+ + Content 3 + + + ` +}) +class AccordionInteractiveContent {} + +@Component({ + selector: 'accordion-nested', + imports: [KbqAccordionModule], + template: ` + + + + + + + + + + + + Inner content 1 + + + + + + Inner content 2 + + + + + + + + + Outer content 2 + + + ` +}) +class AccordionNested { + readonly outerValueChanges: (string[] | string)[] = []; +} + +@Component({ + selector: 'accordion-nested-in-trigger', + imports: [KbqAccordionModule], + template: ` + + + + + + Content + + + ` +}) +class AccordionNestedInTrigger {} diff --git a/packages/components/accordion/accordion.ts b/packages/components/accordion/accordion.ts index 392b8e34c4..5d84aa0a49 100644 --- a/packages/components/accordion/accordion.ts +++ b/packages/components/accordion/accordion.ts @@ -21,6 +21,7 @@ import { numberAttribute, OnDestroy, output, + Signal, untracked, ViewEncapsulation } from '@angular/core'; @@ -50,8 +51,7 @@ let uniqueIdCounter: number = 0; encapsulation: ViewEncapsulation.None, host: { class: 'kbq-accordion', - '[attr.data-orientation]': 'orientation()', - '(keydown)': 'keydownHandler($event)' + '[attr.data-orientation]': 'orientation()' } }) export class KbqAccordion implements OnDestroy, AfterViewInit, AfterContentInit { @@ -75,12 +75,33 @@ export class KbqAccordion implements OnDestroy, AfterViewInit, AfterContentInit /** Emits every time `openAll()` or `closeAll()` is called. @docs-private */ readonly openCloseAllActions = new Subject(); - /** The accordion items projected into this accordion. @docs-private */ - readonly items = contentChildren( + /** + * Every item the content query matches, including those of a nested accordion. + * Use `items` instead — this one is the raw, unfiltered query. + * @docs-private + */ + // The generic is explicit because `forwardRef` erases the locator type, which would leave both + // this query and `items` as `Signal` in the public API report. + protected readonly allItems: Signal = contentChildren( forwardRef(() => KbqAccordionItem), { descendants: true } ); + /** + * The accordion items that belong to this accordion. + * + * A descendant content query resolves against the template the tag is authored in and does not + * stop at a nested component of the same type, so `allItems` of an outer accordion also matches + * the items of an accordion rendered inside an item's content. Each item's own `accordion` is + * injected and therefore always its nearest one, which makes ownership the reliable filter. + * Without it the key manager would move focus into the nested accordion's headers, `valueChange` + * would report a nested item's value and state saving would persist it under the outer key. + * @docs-private + */ + readonly items: Signal = computed(() => + this.allItems().filter((item) => item.accordion === this) + ); + /** Whether the accordion persists the expanded state of its items across reloads. Defaults to `false`. */ readonly useStateSaving = input(false, { transform: booleanAttribute }); @@ -249,20 +270,23 @@ export class KbqAccordion implements OnDestroy, AfterViewInit, AfterContentInit this.keyManager?.destroy(); } - /** @docs-private */ + /** + * Handles a key pressed on an item's trigger. + * + * Invoked by `KbqAccordionTriggerDirective`, not bound on the accordion host: a root listener + * also receives keys bubbling from the section content and from controls placed next to the + * trigger, and would swallow their Enter/Space and hijack their arrow keys. It would also + * activate the first item — stealing focus — the first time any key was pressed anywhere inside. + * @docs-private + */ keydownHandler(event: KeyboardEvent) { - if (!this.keyManager) return; - - if (!this.keyManager.activeItem) { - this.keyManager.setFirstItemActive(); - } + const activeItem = this.keyManager?.activeItem; - const activeItem = this.keyManager.activeItem; + if (!activeItem) return; if ( (event.keyCode === ENTER || event.keyCode === SPACE) && !this.keyManager.isTyping() && - activeItem && !activeItem.disabled ) { event.preventDefault(); diff --git a/packages/components/icon/__screenshots__/02-light.png b/packages/components/icon/__screenshots__/02-light.png index 019e596055..e289e5e83f 100644 Binary files a/packages/components/icon/__screenshots__/02-light.png and b/packages/components/icon/__screenshots__/02-light.png differ diff --git a/packages/docs-examples/components/accordion/accordion-header/accordion-header-example.html b/packages/docs-examples/components/accordion/accordion-header/accordion-header-example.html index 86d06c5fa9..2cccb0ac6c 100644 --- a/packages/docs-examples/components/accordion/accordion-header/accordion-header-example.html +++ b/packages/docs-examples/components/accordion/accordion-header/accordion-header-example.html @@ -1,161 +1,209 @@
- + + @if (actions) { - - - - } - @if (rightBadge) { - - Badge - - } - +
+ + + +
+ } + Данный текст используется для иллюстрации внутреннего содержимого секции аккордеона - + @if (actions) { - - - - } - @if (rightBadge) { - - Badge - - } - +
+ + + +
+ } + Данный текст используется для иллюстрации внутреннего содержимого секции аккордеона - + @if (actions) { - - - - } - @if (rightBadge) { - - Badge - - } - +
+ + + +
+ } + Данный текст используется для иллюстрации внутреннего содержимого секции аккордеона - + @if (actions) { - - - - } - @if (rightBadge) { - - Badge - - } - +
+ + + +
+ } + Данный текст используется для иллюстрации внутреннего содержимого секции аккордеона - + @if (actions) { - - - - } - @if (rightBadge) { - - Badge - - } - +
+ + + +
+ } + Данный текст используется для иллюстрации внутреннего содержимого секции аккордеона diff --git a/packages/docs-examples/components/accordion/accordion-interactive-elements/accordion-interactive-elements-example.html b/packages/docs-examples/components/accordion/accordion-interactive-elements/accordion-interactive-elements-example.html new file mode 100644 index 0000000000..124918b171 --- /dev/null +++ b/packages/docs-examples/components/accordion/accordion-interactive-elements/accordion-interactive-elements-example.html @@ -0,0 +1,53 @@ +
+ + @for (section of sections; track section.id) { + + + + + +
+ + +
+ + + +
+ Enabled + + + + + + +
+ + + } + + + + + + + +
+ +
+ Enter and Space toggle the section only while the trigger itself is focused +
diff --git a/packages/docs-examples/components/accordion/accordion-interactive-elements/accordion-interactive-elements-example.ts b/packages/docs-examples/components/accordion/accordion-interactive-elements/accordion-interactive-elements-example.ts new file mode 100644 index 0000000000..c1f377cc35 --- /dev/null +++ b/packages/docs-examples/components/accordion/accordion-interactive-elements/accordion-interactive-elements-example.ts @@ -0,0 +1,42 @@ +import { ChangeDetectionStrategy, Component } from '@angular/core'; +import { FormsModule } from '@angular/forms'; +import { KbqAccordionModule } from '@koobiq/components/accordion'; +import { KbqButtonModule } from '@koobiq/components/button'; +import { KbqCheckboxModule } from '@koobiq/components/checkbox'; +import { KbqDropdownModule } from '@koobiq/components/dropdown'; +import { KbqFormFieldModule } from '@koobiq/components/form-field'; +import { KbqIconModule } from '@koobiq/components/icon'; +import { KbqInputModule } from '@koobiq/components/input'; + +type ExampleSection = { + id: string; + title: string; + enabled: boolean; + name: string; +}; + +/** + * @title Accordion interactive elements + */ +@Component({ + selector: 'accordion-interactive-elements-example', + imports: [ + KbqAccordionModule, + KbqButtonModule, + KbqCheckboxModule, + KbqDropdownModule, + KbqFormFieldModule, + KbqIconModule, + KbqInputModule, + FormsModule + ], + templateUrl: 'accordion-interactive-elements-example.html', + changeDetection: ChangeDetectionStrategy.OnPush +}) +export class AccordionInteractiveElementsExample { + protected readonly sections: ExampleSection[] = [ + { id: 'section-1', title: 'Profile', enabled: true, name: '' }, + { id: 'section-2', title: 'Specifications', enabled: false, name: '' }, + { id: 'section-3', title: 'Servers', enabled: false, name: '' } + ]; +} diff --git a/packages/docs-examples/components/accordion/index.ts b/packages/docs-examples/components/accordion/index.ts index 211c754f30..0464bf4acf 100644 --- a/packages/docs-examples/components/accordion/index.ts +++ b/packages/docs-examples/components/accordion/index.ts @@ -4,6 +4,7 @@ import { AccordionHeaderExample } from './accordion-header/accordion-header-exam import { AccordionInPanelExample } from './accordion-in-panel/accordion-in-panel-example'; import { AccordionInSectionExample } from './accordion-in-section/accordion-in-section-example'; import { AccordionInactiveSectionExample } from './accordion-inactive-section/accordion-inactive-section-example'; +import { AccordionInteractiveElementsExample } from './accordion-interactive-elements/accordion-interactive-elements-example'; import { AccordionOverviewExample } from './accordion-overview/accordion-overview-example'; import { AccordionSectionsExample } from './accordion-sections/accordion-sections-example'; import { AccordionStatesExample } from './accordion-states/accordion-states-example'; @@ -14,6 +15,7 @@ export { AccordionInactiveSectionExample, AccordionInPanelExample, AccordionInSectionExample, + AccordionInteractiveElementsExample, AccordionOverviewExample, AccordionSectionsExample, AccordionStatesExample @@ -26,6 +28,7 @@ const EXAMPLES = [ AccordionInactiveSectionExample, AccordionHeaderExample, AccordionContentExample, + AccordionInteractiveElementsExample, AccordionInSectionExample, AccordionInPanelExample ]; diff --git a/packages/docs-examples/example-module.ts b/packages/docs-examples/example-module.ts index b48e99838f..e6bf9535d5 100644 --- a/packages/docs-examples/example-module.ts +++ b/packages/docs-examples/example-module.ts @@ -99,6 +99,20 @@ export const EXAMPLE_COMPONENTS: {[id: string]: LiveExample} = { "primaryFile": "accordion-inactive-section-example.ts", "importPath": "components/accordion" }, + "accordion-interactive-elements": { + "packagePath": "components/accordion/accordion-interactive-elements", + "title": "Accordion interactive elements", + "componentName": "AccordionInteractiveElementsExample", + "files": [ + "accordion-interactive-elements-example.ts", + "accordion-interactive-elements-example.html" + ], + "localImportFiles": [], + "selector": "accordion-interactive-elements-example", + "additionalComponents": [], + "primaryFile": "accordion-interactive-elements-example.ts", + "importPath": "components/accordion" + }, "accordion-overview": { "packagePath": "components/accordion/accordion-overview", "title": "Accordion", @@ -7502,6 +7516,8 @@ return import('@koobiq/docs-examples/components/accordion'); case 'accordion-in-section': return import('@koobiq/docs-examples/components/accordion'); case 'accordion-inactive-section': +return import('@koobiq/docs-examples/components/accordion'); + case 'accordion-interactive-elements': return import('@koobiq/docs-examples/components/accordion'); case 'accordion-overview': return import('@koobiq/docs-examples/components/accordion'); diff --git a/tools/public_api_guard/components/accordion.api.md b/tools/public_api_guard/components/accordion.api.md index da5538b189..3ad1c56fcd 100644 --- a/tools/public_api_guard/components/accordion.api.md +++ b/tools/public_api_guard/components/accordion.api.md @@ -16,6 +16,7 @@ import * as i1 from '@koobiq/components/icon'; import { InjectionToken } from '@angular/core'; import { KbqIcon } from '@koobiq/components/icon'; import { OnDestroy } from '@angular/core'; +import { Signal } from '@angular/core'; import { Subject } from 'rxjs'; import { UniqueSelectionDispatcher } from '@angular/cdk/collections'; @@ -25,6 +26,7 @@ export const KBQ_ACCORDION_STATE_STORE: InjectionToken; // @public (undocumented) export class KbqAccordion implements OnDestroy, AfterViewInit, AfterContentInit { constructor(); + protected readonly allItems: Signal; protected readonly changeDetectorRef: ChangeDetectorRef; closeAll(): void; readonly collapsible: _angular_core.InputSignal; @@ -36,7 +38,7 @@ export class KbqAccordion implements OnDestroy, AfterViewInit, AfterContentInit get hasSavedState(): boolean; get id(): string; get isMultiple(): boolean; - readonly items: _angular_core.Signal; + readonly items: Signal; keydownHandler(event: KeyboardEvent): void; protected keyManager: FocusKeyManager; readonly level: _angular_core.InputSignalWithTransform; @@ -55,12 +57,12 @@ export class KbqAccordion implements OnDestroy, AfterViewInit, AfterContentInit readonly stateSavingKey: _angular_core.InputSignal; readonly type: _angular_core.InputSignal; readonly useStateSaving: _angular_core.InputSignalWithTransform; - readonly value: _angular_core.Signal; + readonly value: Signal; readonly valueChange: _angular_core.OutputEmitterRef; readonly valueInput: _angular_core.InputSignal; readonly variant: _angular_core.InputSignal; // (undocumented) - static ɵcmp: _angular_core.ɵɵComponentDeclaration; + static ɵcmp: _angular_core.ɵɵComponentDeclaration; // (undocumented) static ɵfac: _angular_core.ɵɵFactoryDeclaration; } @@ -99,6 +101,7 @@ export class KbqAccordionContentDirective implements AfterViewInit { export class KbqAccordionHeader { protected readonly accordion: KbqAccordion; protected readonly item: KbqAccordionItem; + protected get labelledBy(): string | null; // (undocumented) static ɵdir: _angular_core.ɵɵDirectiveDeclaration; // (undocumented) @@ -108,7 +111,7 @@ export class KbqAccordionHeader { // @public (undocumented) export class KbqAccordionItem implements OnDestroy { constructor(); - protected readonly accordion: KbqAccordion; + readonly accordion: KbqAccordion; protected readonly changeDetectorRef: ChangeDetectorRef; close(): void; readonly closed: _angular_core.OutputEmitterRef; @@ -226,6 +229,7 @@ export class KbqAccordionTriggerDirective { protected readonly nativeElement: HTMLElement; onClick(): void; onFocus(): void; + onKeydown(event: KeyboardEvent): void; get triggerId(): string; // (undocumented) static ɵdir: _angular_core.ɵɵDirectiveDeclaration;