Skip to content

Commit

Permalink
Merge branch 'no-focus-on-hover-27-july' into no-focus-on-hover
Browse files Browse the repository at this point in the history
  • Loading branch information
@gabalafou
gabalafou committed Jul 27, 2023
2 parents 4274b02 + ff4a3c7 commit 97424bf
Show file tree
Hide file tree
Showing 3 changed files with 195 additions and 44 deletions.
13 changes: 13 additions & 0 deletions packages/default-theme/style/menubar.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,16 @@
border-right: 1px solid #c0c0c0;
box-shadow: 0px 0px 6px rgba(0, 0, 0, 0.2);
}

.lm-MenuBar-item:focus {
text-decoration: underline;
}

.lm-MenuBar-item:focus-visible {
outline: 2px solid #333;
outline-offset: -2px;
}

.lm-MenuBar-item[aria-disabled="true"] {
color: rgba(0, 0, 0, 0.37);
}
104 changes: 76 additions & 28 deletions packages/widgets/src/menubar.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@ export class MenuBar extends Widget {
value = -1;
}

// An empty menu cannot be active
if (value > -1 && this._menus[value].items.length === 0) {
value = -1;
}

// Bail early if the index will not change.
if (this._activeIndex === value) {
return;
Expand All @@ -165,11 +170,6 @@ export class MenuBar extends Widget {
// Update the active index.
this._activeIndex = value;

// Update the focus index.
if (value !== -1) {
this._tabFocusIndex = value;
}

// Schedule an update of the items.
this.update();
}
Expand Down Expand Up @@ -422,7 +422,7 @@ export class MenuBar extends Widget {
*/
protected onActivateRequest(msg: Message): void {
if (this.isAttached) {
this._focusItem(0);
this._focusItemAt(0);
}
}

Expand Down Expand Up @@ -457,9 +457,11 @@ export class MenuBar extends Widget {
for (let i = 0; i < length; ++i) {
content[i] = renderer.renderItem({
title: menus[i].title,
active: i === activeIndex && menus[i].items.length !== 0,
active: i === activeIndex,
tabbable: i === tabFocusIndex,
disabled: menus[i].items.length === 0,
onfocus: () => {
this._tabFocusIndex = i;
this.activeIndex = i;
}
});
Expand Down Expand Up @@ -496,7 +498,9 @@ export class MenuBar extends Widget {
title: this._overflowMenu.title,
active: length === activeIndex && menus[length].items.length !== 0,
tabbable: length === tabFocusIndex,
disabled: menus[length].items.length === 0,
onfocus: () => {
this._tabFocusIndex = length;
this.activeIndex = length;
}
});
Expand All @@ -516,7 +520,9 @@ export class MenuBar extends Widget {
title: menu.title,
active: false,
tabbable: length === tabFocusIndex,
disabled: menus[length].items.length === 0,
onfocus: () => {
this._tabFocusIndex = length;
this.activeIndex = length;
}
});
Expand Down Expand Up @@ -596,29 +602,38 @@ export class MenuBar extends Widget {

// Enter, Space, Up Arrow, Down Arrow
if (kc === 13 || kc === 32 || kc === 38 || kc === 40) {
// The active index may have changed (for example, user hovers over an
// item with the mouse), so be sure to use the focus index.
this.activeIndex = this._tabFocusIndex;
if (this.activeIndex !== this._tabFocusIndex) {
// Bail if the setter refused to set activeIndex to tabFocusIndex
// because it means that the item at tabFocusIndex cannot be opened (for
// example, it has an empty menu)
return;
}
this.openActiveMenu();
return;
}

// Escape
if (kc === 27) {
this._closeChildMenu();
this._focusItemAt(this.activeIndex);
return;
}

// Left Arrow
if (kc === 37) {
let i = this._activeIndex;
// Left or Right Arrow
if (kc === 37 || kc === 39) {
let direction = kc === 37 ? -1 : 1;
let start = this._tabFocusIndex + direction;
let n = this._menus.length;
this._focusItem(i === 0 ? n - 1 : i - 1);
return;
}

// Right Arrow
if (kc === 39) {
let i = this._activeIndex;
let n = this._menus.length;
this._focusItem(i === n - 1 ? 0 : i + 1);
for (let i = 0; i < n; i++) {
let index = (n + start + direction * i) % n;
if (this._menus[index].items.length) {
this._focusItemAt(index);
return;
}
}
return;
}

Expand All @@ -642,9 +657,11 @@ export class MenuBar extends Widget {
this.activeIndex = result.index;
this.openActiveMenu();
} else if (result.index !== -1) {
this._focusItem(result.index);
this.activeIndex = result.index;
this._focusItemAt(this.activeIndex);
} else if (result.auto !== -1) {
this._focusItem(result.auto);
this.activeIndex = result.auto;
this._focusItemAt(this.activeIndex);
}
}

Expand Down Expand Up @@ -685,8 +702,8 @@ export class MenuBar extends Widget {
this._closeChildMenu();
this.activeIndex = index;
} else {
// Call preventDefault() so that the menu being opened can take focus,
// rather than the menu opener that the user clicked.
// If we don't call preventDefault() here, then the item in the menu
// bar will take focus over the menu that is being opened.
event.preventDefault();
const position = this._positionForMenu(index);
Menu.saveWindowData();
Expand Down Expand Up @@ -752,12 +769,25 @@ export class MenuBar extends Widget {
* Handle the `'focusout'` event for the menu bar.
*/
private _evtFocusOut(event: FocusEvent): void {
// Reset the active index if menubar is losing focus and there is no open menu.
// Reset the active index if there is no open menu and the menubar is losing focus.
if (!this._childMenu && !this.node.contains(event.relatedTarget as Node)) {
this.activeIndex = -1;
}
}

/**
* Focus an item in the menu bar.
*
* #### Notes
* Does not open the associated menu.
*/
private _focusItemAt(index: number): void {
const itemNode = this.contentNode.childNodes[index] as HTMLElement | void;
if (itemNode) {
itemNode.focus();
}
}

/**
* Open the child menu at the active index immediately.
*
Expand Down Expand Up @@ -788,6 +818,11 @@ export class MenuBar extends Widget {
document.addEventListener('mousedown', this, true);
}

// Update the tab focus index. Note: it's important to follow this with an
// "update-request" message so that the `tabindex` attribute on each menubar
// item gets properly updated.
this._tabFocusIndex = this.activeIndex;

// Ensure the menu bar is updated and look up the item node.
MessageLoop.sendMessage(this, Widget.Msg.UpdateRequest);

Expand Down Expand Up @@ -892,7 +927,7 @@ export class MenuBar extends Widget {
this.update();
}

// Track the index of the item that is currently focused or whose menu is open. -1 means nothing focused.
// Track the index of the item that is currently focused or hovered. -1 means nothing focused or hovered.
private _activeIndex = -1;
// Track which item can be focused using the TAB key. Unlike _activeIndex will always point to a menuitem.
private _tabFocusIndex = 0;
Expand Down Expand Up @@ -959,6 +994,15 @@ export namespace MenuBar {
*/
readonly tabbable: boolean;

/**
* Whether the item is disabled.
*
* #### Notes
* A disabled item cannot be active.
* A disabled item cannot be focussed.
*/
readonly disabled: boolean;

readonly onfocus?: (event: FocusEvent) => void;
}

Expand Down Expand Up @@ -998,7 +1042,7 @@ export namespace MenuBar {
{
className,
dataset,
tabindex: data.tabbable ? '0' : '-1',
...(data.disabled ? {} : { tabindex: data.tabbable ? '0' : '-1' }),
onfocus: data.onfocus,
...aria
},
Expand Down Expand Up @@ -1045,7 +1089,7 @@ export namespace MenuBar {
if (data.title.className) {
name += ` ${data.title.className}`;
}
if (data.active) {
if (data.active && !data.disabled) {
name += ' lm-mod-active';
}
return name;
Expand All @@ -1070,7 +1114,11 @@ export namespace MenuBar {
* @returns The aria attributes object for the item.
*/
createItemARIA(data: IRenderData): ElementARIAAttrs {
return { role: 'menuitem', 'aria-haspopup': 'true' };
return {
role: 'menuitem',
'aria-haspopup': 'true',
'aria-disabled': data.disabled ? 'true' : 'false'
};
}

/**
Expand Down
Loading

0 comments on commit 97424bf

Please sign in to comment.