From 185937003d573e781467aa19ccda96cbe1ddcb9a Mon Sep 17 00:00:00 2001 From: Nicolas Brichet Date: Mon, 13 Feb 2023 14:07:27 +0100 Subject: [PATCH 1/5] Adds IDs to the widgets of accordion panel --- packages/widgets/src/accordionlayout.ts | 8 ++++++++ packages/widgets/src/accordionpanel.ts | 15 ++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/packages/widgets/src/accordionlayout.ts b/packages/widgets/src/accordionlayout.ts index a4247605f..dd64619c2 100644 --- a/packages/widgets/src/accordionlayout.ts +++ b/packages/widgets/src/accordionlayout.ts @@ -223,6 +223,14 @@ export namespace AccordionLayout { * @returns A element representing the section title. */ createSectionTitle(title: Title): HTMLElement; + + /** + * Ensures the existence of the widget ID. + * It is useful to link the widget node to the 'aria-controls' attribute of the title. + * + * @param widget - The widget concerned. + */ + addWidgetId(widget: Widget): void; } } diff --git a/packages/widgets/src/accordionpanel.ts b/packages/widgets/src/accordionpanel.ts index 79a4c1b79..9e691e604 100644 --- a/packages/widgets/src/accordionpanel.ts +++ b/packages/widgets/src/accordionpanel.ts @@ -62,6 +62,7 @@ export class AccordionPanel extends SplitPanel { * If the widget is already contained in the panel, it will be moved. */ addWidget(widget: Widget): void { + this.renderer.addWidgetId(widget); super.addWidget(widget); widget.title.changed.connect(this._onTitleChanged, this); } @@ -401,7 +402,6 @@ export namespace AccordionPanel { */ createSectionTitle(data: Title): HTMLElement { const handle = document.createElement('h3'); - handle.setAttribute('role', 'tab'); handle.setAttribute('tabindex', '0'); handle.id = this.createTitleKey(data); handle.className = this.titleClassName; @@ -440,9 +440,22 @@ export namespace AccordionPanel { return key; } + /** + * Ensures the existence of the widget ID. + * It is useful to link the widget node to the 'aria-controls' attribute of the title. + * + * @param widget - The widget concerned. + */ + addWidgetId(widget: Widget): void { + if (!widget.id) { + widget.id = `accordion-item-${this._uuid}-${this._widgetID++}`; + } + } + private static _nInstance = 0; private readonly _uuid: number; private _titleID = 0; + private _widgetID = 0; private _titleKeys = new WeakMap, string>(); } From e54b19e6401c3e6537889946b9bd8e6f3dfb4528 Mon Sep 17 00:00:00 2001 From: Nicolas Brichet Date: Mon, 13 Feb 2023 14:47:54 +0100 Subject: [PATCH 2/5] Fix test installation --- packages/widgets/tests/src/accordionlayout.spec.ts | 3 ++- packages/widgets/tests/src/accordionpanel.spec.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/widgets/tests/src/accordionlayout.spec.ts b/packages/widgets/tests/src/accordionlayout.spec.ts index 8a999b2ef..7f62223ce 100644 --- a/packages/widgets/tests/src/accordionlayout.spec.ts +++ b/packages/widgets/tests/src/accordionlayout.spec.ts @@ -9,7 +9,8 @@ import { expect } from 'chai'; const renderer: AccordionLayout.IRenderer = { titleClassName: '.lm-AccordionTitle', createHandle: () => document.createElement('div'), - createSectionTitle: (title: Title) => document.createElement('h3') + createSectionTitle: (title: Title) => document.createElement('h3'), + addWidgetId: (widget: Widget) => (widget.id = 'accordion-test-id') }; class LogAccordionLayout extends AccordionLayout { diff --git a/packages/widgets/tests/src/accordionpanel.spec.ts b/packages/widgets/tests/src/accordionpanel.spec.ts index 869e6079d..ceaea084d 100644 --- a/packages/widgets/tests/src/accordionpanel.spec.ts +++ b/packages/widgets/tests/src/accordionpanel.spec.ts @@ -15,7 +15,8 @@ const bubbles = true; const renderer: AccordionPanel.IRenderer = { titleClassName: '.lm-AccordionTitle', createHandle: () => document.createElement('div'), - createSectionTitle: (title: Title) => document.createElement('h3') + createSectionTitle: (title: Title) => document.createElement('h3'), + addWidgetId: (widget: Widget) => (widget.id = 'accordion-test-id') }; class LogAccordionPanel extends AccordionPanel { From 1c5fe097922720548395af1bb08aafa7e16b8492 Mon Sep 17 00:00:00 2001 From: Nicolas Brichet Date: Tue, 14 Feb 2023 10:25:45 +0100 Subject: [PATCH 3/5] Reverts API changes --- packages/widgets/src/accordionlayout.ts | 31 ++++++++++++++----- packages/widgets/src/accordionpanel.ts | 14 --------- .../widgets/tests/src/accordionlayout.spec.ts | 3 +- .../widgets/tests/src/accordionpanel.spec.ts | 3 +- 4 files changed, 25 insertions(+), 26 deletions(-) diff --git a/packages/widgets/src/accordionlayout.ts b/packages/widgets/src/accordionlayout.ts index dd64619c2..fa2da95dc 100644 --- a/packages/widgets/src/accordionlayout.ts +++ b/packages/widgets/src/accordionlayout.ts @@ -4,6 +4,7 @@ */ import { ArrayExt } from '@lumino/algorithm'; +import { UUID } from '@lumino/coreutils'; import { SplitLayout } from './splitlayout'; import { Title } from './title'; import Utils from './utils'; @@ -83,6 +84,28 @@ export class AccordionLayout extends SplitLayout { this.parent!.node.replaceChild(newTitle, oldTitle); } + /** + * Insert a widget into the layout at the specified index. + * + * @param index - The index at which to insert the widget. + * + * @param widget - The widget to insert into the layout. + * + * #### Notes + * The index will be clamped to the bounds of the widgets. + * + * If the widget is already added to the layout, it will be moved. + * + * #### Undefined Behavior + * An `index` which is non-integral. + */ + insertWidget(index: number, widget: Widget): void { + if (!widget.id) { + widget.id = `id-${UUID.uuid4()}`; + } + super.insertWidget(index, widget); + } + /** * Attach a widget to the parent's DOM node. * @@ -223,14 +246,6 @@ export namespace AccordionLayout { * @returns A element representing the section title. */ createSectionTitle(title: Title): HTMLElement; - - /** - * Ensures the existence of the widget ID. - * It is useful to link the widget node to the 'aria-controls' attribute of the title. - * - * @param widget - The widget concerned. - */ - addWidgetId(widget: Widget): void; } } diff --git a/packages/widgets/src/accordionpanel.ts b/packages/widgets/src/accordionpanel.ts index 9e691e604..37bee5c6c 100644 --- a/packages/widgets/src/accordionpanel.ts +++ b/packages/widgets/src/accordionpanel.ts @@ -62,7 +62,6 @@ export class AccordionPanel extends SplitPanel { * If the widget is already contained in the panel, it will be moved. */ addWidget(widget: Widget): void { - this.renderer.addWidgetId(widget); super.addWidget(widget); widget.title.changed.connect(this._onTitleChanged, this); } @@ -440,22 +439,9 @@ export namespace AccordionPanel { return key; } - /** - * Ensures the existence of the widget ID. - * It is useful to link the widget node to the 'aria-controls' attribute of the title. - * - * @param widget - The widget concerned. - */ - addWidgetId(widget: Widget): void { - if (!widget.id) { - widget.id = `accordion-item-${this._uuid}-${this._widgetID++}`; - } - } - private static _nInstance = 0; private readonly _uuid: number; private _titleID = 0; - private _widgetID = 0; private _titleKeys = new WeakMap, string>(); } diff --git a/packages/widgets/tests/src/accordionlayout.spec.ts b/packages/widgets/tests/src/accordionlayout.spec.ts index 7f62223ce..8a999b2ef 100644 --- a/packages/widgets/tests/src/accordionlayout.spec.ts +++ b/packages/widgets/tests/src/accordionlayout.spec.ts @@ -9,8 +9,7 @@ import { expect } from 'chai'; const renderer: AccordionLayout.IRenderer = { titleClassName: '.lm-AccordionTitle', createHandle: () => document.createElement('div'), - createSectionTitle: (title: Title) => document.createElement('h3'), - addWidgetId: (widget: Widget) => (widget.id = 'accordion-test-id') + createSectionTitle: (title: Title) => document.createElement('h3') }; class LogAccordionLayout extends AccordionLayout { diff --git a/packages/widgets/tests/src/accordionpanel.spec.ts b/packages/widgets/tests/src/accordionpanel.spec.ts index ceaea084d..869e6079d 100644 --- a/packages/widgets/tests/src/accordionpanel.spec.ts +++ b/packages/widgets/tests/src/accordionpanel.spec.ts @@ -15,8 +15,7 @@ const bubbles = true; const renderer: AccordionPanel.IRenderer = { titleClassName: '.lm-AccordionTitle', createHandle: () => document.createElement('div'), - createSectionTitle: (title: Title) => document.createElement('h3'), - addWidgetId: (widget: Widget) => (widget.id = 'accordion-test-id') + createSectionTitle: (title: Title) => document.createElement('h3') }; class LogAccordionPanel extends AccordionPanel { From 2360bb4c77d49d0688574088c851ee86de633e8a Mon Sep 17 00:00:00 2001 From: Nicolas Brichet Date: Tue, 14 Feb 2023 11:25:45 +0100 Subject: [PATCH 4/5] Add minimal tests on accessibility --- .../widgets/tests/src/accordionpanel.spec.ts | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/packages/widgets/tests/src/accordionpanel.spec.ts b/packages/widgets/tests/src/accordionpanel.spec.ts index 869e6079d..05bdb72d6 100644 --- a/packages/widgets/tests/src/accordionpanel.spec.ts +++ b/packages/widgets/tests/src/accordionpanel.spec.ts @@ -77,6 +77,37 @@ describe('@lumino/widgets', () => { }); }); + describe('#accessibility()', () => { + it('should create a widget ID if it does not exist', () => { + let panel = new LogAccordionPanel(); + let w1 = new Widget(); + let w2 = new Widget(); + + // Expects a widget ID to be created. + expect(!w1.id); + panel.addWidget(w1); + expect(w1.id); + + // Expects the widget ID to be unchanged. + w2.id = 'test_id'; + panel.addWidget(w2); + expect(w2.id === 'test_id'); + }); + + it('should link the widget to its title', () => { + let layout = new AccordionLayout({ renderer }); + let panel = new LogAccordionPanel({ layout }); + let w = new Widget(); + panel.addWidget(w); + + expect(layout.titles[0].getAttribute('aria-controls') === w.id); + expect( + layout.widgets[0].node.getAttribute('aria-labelledby') === + layout.titles[0].id + ); + }); + }); + describe('#dispose()', () => { it('should dispose of the resources held by the panel', () => { let panel = new LogAccordionPanel(); From 8e4e0d6937783c8419eac66869607d8ac5deca2c Mon Sep 17 00:00:00 2001 From: Nicolas Brichet Date: Tue, 14 Feb 2023 17:14:41 +0100 Subject: [PATCH 5/5] Updates API description and adds assert to tests --- packages/widgets/tests/src/accordionpanel.spec.ts | 15 +++++++-------- review/api/widgets.api.md | 1 + 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/widgets/tests/src/accordionpanel.spec.ts b/packages/widgets/tests/src/accordionpanel.spec.ts index 05bdb72d6..b93a22bdc 100644 --- a/packages/widgets/tests/src/accordionpanel.spec.ts +++ b/packages/widgets/tests/src/accordionpanel.spec.ts @@ -84,14 +84,14 @@ describe('@lumino/widgets', () => { let w2 = new Widget(); // Expects a widget ID to be created. - expect(!w1.id); + expect(w1.id).to.be.empty; panel.addWidget(w1); - expect(w1.id); + expect(w1.id).to.match(/id-[0-9a-z\-]+/); // Expects the widget ID to be unchanged. - w2.id = 'test_id'; + w2.id = 'test-id'; panel.addWidget(w2); - expect(w2.id === 'test_id'); + expect(w2.id).to.equal('test-id'); }); it('should link the widget to its title', () => { @@ -100,10 +100,9 @@ describe('@lumino/widgets', () => { let w = new Widget(); panel.addWidget(w); - expect(layout.titles[0].getAttribute('aria-controls') === w.id); - expect( - layout.widgets[0].node.getAttribute('aria-labelledby') === - layout.titles[0].id + expect(layout.titles[0].getAttribute('aria-controls')).to.equal(w.id); + expect(layout.widgets[0].node.getAttribute('aria-labelledby')).to.equal( + layout.titles[0].id ); }); }); diff --git a/review/api/widgets.api.md b/review/api/widgets.api.md index fca3f1f82..de211d650 100644 --- a/review/api/widgets.api.md +++ b/review/api/widgets.api.md @@ -24,6 +24,7 @@ export class AccordionLayout extends SplitLayout { protected attachWidget(index: number, widget: Widget): void; protected detachWidget(index: number, widget: Widget): void; dispose(): void; + insertWidget(index: number, widget: Widget): void; protected moveWidget(fromIndex: number, toIndex: number, widget: Widget): void; readonly renderer: AccordionLayout.IRenderer; get titles(): ReadonlyArray;