Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

[accessibility] Uses the arrow keys for tab bar navigation #612

Merged
merged 12 commits into from
Jul 27, 2023
Merged

[accessibility] Uses the arrow keys for tab bar navigation #612

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
12a181b
Uses the arrow keys for tab bar navigation
brichet Jul 20, 2023
306e71a
Add tests
brichet Jul 20, 2023
37810b6
Add tests on vertical tab bar
brichet Jul 20, 2023
ddcad85
Update api
brichet Jul 20, 2023
6033487
typo
brichet Jul 20, 2023
8ab2fa5
Update packages/widgets/src/tabbar.ts
brichet Jul 20, 2023
49796cd
Update packages/widgets/src/tabbar.ts
brichet Jul 20, 2023
6571ef1
Update packages/widgets/src/tabbar.ts
brichet Jul 20, 2023
9d395d8
Set a default tabindex for tabs and avoid looping on tabs to update t…
brichet Jul 20, 2023
2bb3466
Fix arrow keys and click on editable title input
brichet Jul 20, 2023
705b918
Manage 'Home' and 'End' keys
brichet Jul 21, 2023
ce2b280
Add tests on titles editable
brichet Jul 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 85 additions & 6 deletions packages/widgets/src/tabbar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import { Title } from './title';

import { Widget } from './widget';

const ARROW_KEYS = ['ArrowLeft', 'ArrowUp', 'ArrowRight', 'ArrowDown'];

/**
* A widget which displays titles as a single row or column of tabs.
*
Expand Down Expand Up @@ -639,15 +641,43 @@ export class TabBar<T> extends Widget {
let renderer = this.renderer;
let currentTitle = this.currentTitle;
let content = new Array<VirtualElement>(titles.length);
// Keep the tabindex="0" attribute to the tab which handled it before the update.
// If the add button handles it, no need to do anything. If no element of the tab
// bar handles it, set it on the current or the first tab to ensure one element
// handles it after update.
const tabHandlingTabindex =
this._getCurrentTabindex() ??
(this._currentIndex > -1 ? this._currentIndex : 0);

for (let i = 0, n = titles.length; i < n; ++i) {
let title = titles[i];
let current = title === currentTitle;
let zIndex = current ? n : n - i - 1;
content[i] = renderer.renderTab({ title, current, zIndex });
let tabIndex = tabHandlingTabindex === i ? 0 : -1;
content[i] = renderer.renderTab({ title, current, zIndex, tabIndex });
}
VirtualDOM.render(content, this.contentNode);
}

/**
* Get the index of the tab which handles tabindex="0".
* If the add button handles tabindex="0", -1 is returned.
* If none of the previous handles tabindex="0", null is returned.
*/
private _getCurrentTabindex(): number | null {
let index = null;
const elemTabindex = this.contentNode.querySelector('li[tabindex="0"]');
if (elemTabindex) {
index = [...this.contentNode.children].indexOf(elemTabindex);
} else if (
this._addButtonEnabled &&
this.addButtonNode.getAttribute('tabindex') === '0'
) {
index = -1;
}
return index;
}

/**
* Handle the `'dblclick'` event for the tab bar.
*/
Expand Down Expand Up @@ -724,7 +754,7 @@ export class TabBar<T> extends Widget {
event.stopPropagation();

// Release the mouse if `Escape` is pressed.
if (event.keyCode === 27) {
if (event.key === 'Escape') {
this._releaseMouse();
}
}
Expand Down Expand Up @@ -765,6 +795,47 @@ export class TabBar<T> extends Widget {
this.currentIndex = index;
}
}
// Handle the arrow keys to switch tabs.
} else if (ARROW_KEYS.includes(event.key)) {
// Create a list of all focusable elements in the tab bar.
const focusable: Element[] = [...this.contentNode.children];
if (this.addButtonEnabled) {
focusable.push(this.addButtonNode);
}
// If the tab bac contains only one element, nothing to do.
brichet marked this conversation as resolved.
Show resolved Hide resolved
if (focusable.length <= 1) {
return;
}
event.preventDefault();
event.stopPropagation();
Comment on lines +818 to +819
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the event being stopped even if there is only one element?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thinks so, to avoid different behavior depending on the number of elements.


// Get the current focused element.
let focusedIndex = focusable.indexOf(document.activeElement as Element);
if (focusedIndex === -1) {
focusedIndex = this._currentIndex;
}

// Find the next element to focus on.
let nextFocused: Element | null | undefined;
if (
(event.key === 'ArrowRight' && this._orientation === 'horizontal') ||
(event.key === 'ArrowDown' && this._orientation === 'vertical')
) {
nextFocused = focusable[focusedIndex + 1] || focusable[0];
brichet marked this conversation as resolved.
Show resolved Hide resolved
} else if (
(event.key === 'ArrowLeft' && this._orientation === 'horizontal') ||
(event.key === 'ArrowUp' && this._orientation === 'vertical')
) {
nextFocused =
focusable[focusedIndex - 1] || focusable[focusable.length - 1];
brichet marked this conversation as resolved.
Show resolved Hide resolved
}

// Change the focused element and the tabindex value.
if (nextFocused) {
focusable.forEach(element => element.setAttribute('tabindex', '-1'));
brichet marked this conversation as resolved.
Show resolved Hide resolved
nextFocused?.setAttribute('tabindex', '0');
(nextFocused as HTMLElement).focus();
}
}
}

Expand Down Expand Up @@ -1555,6 +1626,11 @@ export namespace TabBar {
* The z-index for the tab.
*/
readonly zIndex: number;

/**
* The tabindex value for the tab.
*/
readonly tabIndex?: number;
}

/**
Expand Down Expand Up @@ -1731,11 +1807,14 @@ export namespace TabBar {
* @returns The ARIA attributes for the tab.
*/
createTabARIA(data: IRenderData<any>): ElementARIAAttrs | ElementBaseAttrs {
return {
const ariaAttributes: { [k: string]: string } = {
brichet marked this conversation as resolved.
Show resolved Hide resolved
role: 'tab',
'aria-selected': data.current.toString(),
tabindex: '0'
'aria-selected': data.current.toString()
};
if (data.tabIndex !== undefined) {
ariaAttributes.tabindex = `${data.tabIndex}`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case this it is undefined should it be set to some default value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having tabindex="-1" as default would probably make sense for a tab.

I'm not quite sure about the difference between tabindex="-1" and tabindex not set.
It seems that the behavior is not the same for input elements and other elements.
In the case of the tab, not setting the tabindex would make it not focusable with tabulation, nor 'programatically'. I'm not sure how useful it can be...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure about the difference between tabindex="-1" and tabindex not set.
It seems that the behavior is not the same for input elements and other elements.

when tabindex is not defined, interactive elements automatically receive tab focus, while non-interactive elements don't. so if you wan't to avoid a tab on an interactive widget you'd use tabindex=-1. if you want to add a non-interactive dom element to the tab order then you'd use tabindex=0.

the ARIA authoring guide example for this pattern uses buttons for the tab element which means they need control what doesn't get tabbed to; the implementation needs to choose one tab element to have tabindex=0, or undefined, and the rest are tabindex=-1. if the tab elements are made of non-interactive elements then the task is to make elements appear in the dom order by adding tabindex=0 to one element.

In the case of the tab, not setting the tabindex would make it not focusable with tabulation, nor 'programatically'. I'm not sure how useful it can be...

hopefully folks don't mess with tab order. that should be private. most recommendations say to avoid tabindex>0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 9d395d8

}
return ariaAttributes;
}

/**
Expand Down Expand Up @@ -1910,7 +1989,7 @@ namespace Private {

let add = document.createElement('div');
add.className = 'lm-TabBar-addButton lm-mod-hidden';
add.setAttribute('tabindex', '0');
add.setAttribute('tabindex', '-1');
node.appendChild(add);
return node;
}
Expand Down
204 changes: 204 additions & 0 deletions packages/widgets/tests/src/tabbar.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1592,6 +1592,210 @@ describe('@lumino/widgets', () => {

expect(addRequested).to.be.true;
});

it('should have the tabindex="0" on the first tab by default', () => {
populateBar(bar);
const firstTab = bar.contentNode.firstChild as HTMLElement;
expect(firstTab.getAttribute('tabindex')).to.equal('0');
for (let i = 1; i < bar.titles.length; i++) {
let tab = bar.contentNode.children[i] as HTMLElement;
expect(tab.getAttribute('tabindex')).to.equal('-1');
}
expect(bar.addButtonNode.getAttribute('tabindex')).to.equal('-1');
});

it('should switch the focus on the second tab on right arrow keydown', () => {
populateBar(bar);
const firstTab = bar.contentNode.firstChild as HTMLElement;
firstTab.focus();
bar.node.dispatchEvent(
new KeyboardEvent('keydown', {
key: 'ArrowRight',
cancelable: true,
bubbles: true
})
);
expect(firstTab.getAttribute('tabindex')).to.equal('-1');
const secondTab = bar.contentNode.children[1] as HTMLElement;
expect(secondTab.getAttribute('tabindex')).to.equal('0');
expect(document.activeElement).to.equal(secondTab);
});

it('should switch the focus on the last tab on left arrow keydown', () => {
populateBar(bar);
const firstTab = bar.contentNode.firstChild as HTMLElement;
firstTab.focus();
bar.node.dispatchEvent(
new KeyboardEvent('keydown', {
key: 'ArrowLeft',
cancelable: true,
bubbles: true
})
);
expect(firstTab.getAttribute('tabindex')).to.equal('-1');
const lastTab = bar.contentNode.lastChild as HTMLElement;
expect(lastTab.getAttribute('tabindex')).to.equal('0');
expect(document.activeElement).to.equal(lastTab);
});

it('should switch the focus on the add button on left arrow keydown', () => {
bar.addButtonEnabled = true;
populateBar(bar);
const firstTab = bar.contentNode.firstChild as HTMLElement;
firstTab.focus();
bar.node.dispatchEvent(
new KeyboardEvent('keydown', {
key: 'ArrowLeft',
cancelable: true,
bubbles: true
})
);
expect(firstTab.getAttribute('tabindex')).to.equal('-1');
expect(bar.addButtonNode.getAttribute('tabindex')).to.equal('0');
expect(document.activeElement).to.equal(bar.addButtonNode);
});

it('should be no-op on up and down arrow keydown', () => {
populateBar(bar);
const firstTab = bar.contentNode.firstChild as HTMLElement;
firstTab.focus();
bar.node.dispatchEvent(
new KeyboardEvent('keydown', {
key: 'ArrowUp',
cancelable: true,
bubbles: true
})
);
expect(firstTab.getAttribute('tabindex')).to.equal('0');
expect(document.activeElement).to.equal(firstTab);
bar.node.dispatchEvent(
new KeyboardEvent('keydown', {
key: 'ArrowDown',
cancelable: true,
bubbles: true
})
);
expect(firstTab.getAttribute('tabindex')).to.equal('0');
expect(document.activeElement).to.equal(firstTab);
});

it('should switch the focus on the second tab on down arrow keydown', () => {
bar.orientation = 'vertical';
populateBar(bar);
const firstTab = bar.contentNode.firstChild as HTMLElement;
firstTab.focus();
bar.node.dispatchEvent(
new KeyboardEvent('keydown', {
key: 'ArrowDown',
cancelable: true,
bubbles: true
})
);
expect(firstTab.getAttribute('tabindex')).to.equal('-1');
const secondTab = bar.contentNode.children[1] as HTMLElement;
expect(secondTab.getAttribute('tabindex')).to.equal('0');
expect(document.activeElement).to.equal(secondTab);
});

it('should switch the focus on the last tab on up arrow keydown', () => {
bar.orientation = 'vertical';
populateBar(bar);
const firstTab = bar.contentNode.firstChild as HTMLElement;
firstTab.focus();
bar.node.dispatchEvent(
new KeyboardEvent('keydown', {
key: 'ArrowUp',
cancelable: true,
bubbles: true
})
);
expect(firstTab.getAttribute('tabindex')).to.equal('-1');
const lastTab = bar.contentNode.lastChild as HTMLElement;
expect(lastTab.getAttribute('tabindex')).to.equal('0');
expect(document.activeElement).to.equal(lastTab);
});

it('should be no-op on left and right arrow keydown', () => {
bar.orientation = 'vertical';
populateBar(bar);
const firstTab = bar.contentNode.firstChild as HTMLElement;
firstTab.focus();
bar.node.dispatchEvent(
new KeyboardEvent('keydown', {
key: 'ArrowLeft',
cancelable: true,
bubbles: true
})
);
expect(firstTab.getAttribute('tabindex')).to.equal('0');
expect(document.activeElement).to.equal(firstTab);
bar.node.dispatchEvent(
new KeyboardEvent('keydown', {
key: 'ArrowRight',
cancelable: true,
bubbles: true
})
);
expect(firstTab.getAttribute('tabindex')).to.equal('0');
expect(document.activeElement).to.equal(firstTab);
});

it('should not change the tabindex values on focusing another element', () => {
const node = document.createElement('div');
node.setAttribute('tabindex', '0');
document.body.append(node);
populateBar(bar);
const firstTab = bar.contentNode.firstChild as HTMLElement;
firstTab.focus();
bar.node.dispatchEvent(
new KeyboardEvent('keydown', {
key: 'ArrowRight',
cancelable: true,
bubbles: true
})
);
node.focus();
const secondTab = bar.contentNode.children[1] as HTMLElement;
expect(document.activeElement).not.to.equal(secondTab);
expect(secondTab.getAttribute('tabindex')).to.equal('0');
});

/**
* This test is skipped as it seems there is no way to trigger a change of focus
* when simulating tabulation keydown.
*
* TODO:
* Find a way to trigger the change of focus.
*/
it.skip('should keep focus on the second tab on tabulation', () => {
const node = document.createElement('div');
node.setAttribute('tabindex', '0');
document.body.append(node);
populateBar(bar);
const firstTab = bar.contentNode.firstChild as HTMLElement;
firstTab.focus();
bar.node.dispatchEvent(
new KeyboardEvent('keydown', {
key: 'ArrowRight',
cancelable: true,
bubbles: true
})
);
bar.node.dispatchEvent(
new KeyboardEvent('keydown', {
key: 'Tab'
})
);
const secondTab = bar.contentNode.children[1] as HTMLElement;
expect(document.activeElement).not.to.equal(secondTab);
bar.node.dispatchEvent(
new KeyboardEvent('keydown', {
key: 'Tab',
shiftKey: true
})
);
expect(document.activeElement).to.equal(secondTab);
});
});

context('contextmenu', () => {
Expand Down
1 change: 1 addition & 0 deletions review/api/widgets.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1115,6 +1115,7 @@ export namespace TabBar {
}
export interface IRenderData<T> {
readonly current: boolean;
readonly tabIndex?: number;
readonly title: Title<T>;
readonly zIndex: number;
}
Expand Down
Loading