Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Commit

Permalink
fix(menu): Remove code to focus on first/last element on TAB/SHIFT+TA…
Browse files Browse the repository at this point in the history 
…B. (#4786)

Instead, TAB should close the menu and respect default browser tab order.

BREAKING CHANGE: The following adapter methods were removed: isFirstElementFocused, isLastElementFocused, focusFirstElement, focusLastElement. The following functionality to handle TAB on menusurface has been removed: "If TAB and last element is focused => Focus on first element", "If SHIFT + TAB and first element is focused => Focus on last element"
  • Loading branch information
@joyzhong
joyzhong authored Jun 4, 2019
1 parent 04aa6ff commit 99af567
Show file tree
Hide file tree
Showing 6 changed files with 4 additions and 174 deletions.
4 changes: 0 additions & 4 deletions packages/mdc-menu-surface/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,6 @@ Method Signature | Description
`isFocused() => boolean` | Returns a boolean value indicating whether the root element of the menu surface is focused.
`saveFocus() => void` | Stores the currently focused element on the document, for restoring with `restoreFocus`.
`restoreFocus() => void` | Restores the previously saved focus state, by making the previously focused element the active focus again.
`isFirstElementFocused() => boolean` | Returns a boolean value indicating if the first focusable element of the menu-surface is focused.
`isLastElementFocused() => boolean` | Returns a boolean value indicating if the last focusable element of the menu-surface is focused.
`focusFirstElement() => void` | Focuses the first focusable element of the menu-surface.
`focusLastElement() => void` | Focuses the last focusable element of the menu-surface.
`getInnerDimensions() => MDCMenuDimensions` | Returns an object with the items container width and height.
`getAnchorDimensions() => ClientRect \| null` | Returns an object with the dimensions and position of the anchor.
`getBodyDimensions() => MDCMenuDimensions` | Returns an object with width and height of the body, in pixels.
Expand Down
8 changes: 0 additions & 8 deletions packages/mdc-menu-surface/adapter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ export interface MDCMenuSurfaceAdapter {

isElementInContainer(el: Element): boolean;
isFocused(): boolean;
isFirstElementFocused(): boolean;
isLastElementFocused(): boolean;
isRtl(): boolean;

getInnerDimensions(): MDCMenuDimensions;
Expand All @@ -57,12 +55,6 @@ export interface MDCMenuSurfaceAdapter {
/** Restores focus to the element that was focused before the menu surface was opened. */
restoreFocus(): void;

/** Focuses the first focusable element in the menu-surface. */
focusFirstElement(): void;

/** Focuses the first focusable element in the menu-surface. */
focusLastElement(): void;

/** Emits an event when the menu surface is closed. */
notifyClose(): void;

Expand Down
13 changes: 0 additions & 13 deletions packages/mdc-menu-surface/component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ export class MDCMenuSurface extends MDCComponent<MDCMenuSurfaceFoundation> {
protected root_!: HTMLElement; // assigned in MDCComponent constructor

private previousFocus_?: HTMLElement | SVGElement | null;
private firstFocusableElement_?: HTMLElement | SVGElement;
private lastFocusableElement_?: HTMLElement | SVGElement;

private handleKeydown_!: SpecificEventListener<'keydown'>; // assigned in initialSyncWithDOM()
private handleBodyClick_!: SpecificEventListener<'click'>; // assigned in initialSyncWithDOM()
Expand Down Expand Up @@ -84,9 +82,6 @@ export class MDCMenuSurface extends MDCComponent<MDCMenuSurfaceFoundation> {

set open(value: boolean) {
if (value) {
const focusableElements = this.root_.querySelectorAll<HTMLElement | SVGElement>(strings.FOCUSABLE_ELEMENTS);
this.firstFocusableElement_ = focusableElements[0];
this.lastFocusableElement_ = focusableElements[focusableElements.length - 1];
this.foundation_.open();
} else {
this.foundation_.close();
Expand Down Expand Up @@ -173,14 +168,6 @@ export class MDCMenuSurface extends MDCComponent<MDCMenuSurfaceFoundation> {
}
}
},
isFirstElementFocused: () =>
this.firstFocusableElement_ ? this.firstFocusableElement_ === document.activeElement : false,
isLastElementFocused: () =>
this.lastFocusableElement_ ? this.lastFocusableElement_ === document.activeElement : false,
focusFirstElement: () =>
this.firstFocusableElement_ && this.firstFocusableElement_.focus && this.firstFocusableElement_.focus(),
focusLastElement: () =>
this.lastFocusableElement_ && this.lastFocusableElement_.focus && this.lastFocusableElement_.focus(),

getInnerDimensions: () => {
return {width: this.root_.offsetWidth, height: this.root_.offsetHeight};
Expand Down
16 changes: 1 addition & 15 deletions packages/mdc-menu-surface/foundation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ export class MDCMenuSurfaceFoundation extends MDCFoundation<MDCMenuSurfaceAdapte

isElementInContainer: () => false,
isFocused: () => false,
isFirstElementFocused: () => false,
isLastElementFocused: () => false,
isRtl: () => false,

getInnerDimensions: () => ({height: 0, width: 0}),
Expand All @@ -80,8 +78,6 @@ export class MDCMenuSurfaceFoundation extends MDCFoundation<MDCMenuSurfaceAdapte

saveFocus: () => undefined,
restoreFocus: () => undefined,
focusFirstElement: () => undefined,
focusLastElement: () => undefined,

notifyClose: () => undefined,
notifyOpen: () => undefined,
Expand Down Expand Up @@ -233,21 +229,11 @@ export class MDCMenuSurfaceFoundation extends MDCFoundation<MDCMenuSurfaceAdapte

/** Handle keys that close the surface. */
handleKeydown(evt: KeyboardEvent) {
const {keyCode, key, shiftKey} = evt;
const {keyCode, key} = evt;

const isEscape = key === 'Escape' || keyCode === 27;
const isTab = key === 'Tab' || keyCode === 9;

if (isEscape) {
this.close();
} else if (isTab) {
if (this.adapter_.isLastElementFocused() && !shiftKey) {
this.adapter_.focusFirstElement();
evt.preventDefault();
} else if (this.adapter_.isFirstElementFocused() && shiftKey) {
this.adapter_.focusLastElement();
evt.preventDefault();
}
}
}

Expand Down
84 changes: 0 additions & 84 deletions test/unit/mdc-menu-surface/mdc-menu-surface.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,58 +309,6 @@ test('adapter#isFocused returns whether the menu surface is focused', () => {
document.body.removeChild(root);
});

test('adapter#isFirstElementFocused returns true if the first element is focused', () => {
const {root, component} = setupTest({open: true});
const item = root.querySelectorAll(strings.FOCUSABLE_ELEMENTS)[0];
document.body.appendChild(root);
component.open = true;

assert.isFalse(component.getDefaultFoundation().adapter_.isFirstElementFocused());
item.focus();
assert.isTrue(component.getDefaultFoundation().adapter_.isFirstElementFocused());
component.open = false;

document.body.removeChild(root);
});

test('adapter#isLastElementFocused returns true if the last element is focused', () => {
const {root, component} = setupTest({open: true});
const item = root.querySelectorAll(strings.FOCUSABLE_ELEMENTS)[1];
document.body.appendChild(root);
component.open = true;

assert.isFalse(component.getDefaultFoundation().adapter_.isLastElementFocused());
item.focus();
assert.isTrue(component.getDefaultFoundation().adapter_.isLastElementFocused());

component.open = false;
document.body.removeChild(root);
});

test('adapter#focusFirstElement focuses the first menu surface element', () => {
const {root, component} = setupTest({open: true});
const item = root.querySelectorAll(strings.FOCUSABLE_ELEMENTS)[0];
document.body.appendChild(root);
component.open = true;

component.getDefaultFoundation().adapter_.focusFirstElement();
assert.equal(document.activeElement, item);
component.open = false;
document.body.removeChild(root);
});

test('adapter#focusLastElement focuses the last menu surface element', () => {
const {root, component} = setupTest({open: true});
const item = root.querySelectorAll(strings.FOCUSABLE_ELEMENTS)[1];
document.body.appendChild(root);
component.open = true;

component.getDefaultFoundation().adapter_.focusLastElement();
assert.equal(document.activeElement, item);
component.open = false;
document.body.removeChild(root);
});

test('adapter#hasAnchor returns true if the menu surface has an anchor', () => {
const anchor = bel`<div class="mdc-menu-surface--anchor"></div>`;
const {root, component} = setupTest({open: true});
Expand Down Expand Up @@ -509,35 +457,3 @@ test('adapter#setMaxHeight sets the maxHeight style on the menu surface element'
component.getDefaultFoundation().adapter_.setMaxHeight('100px');
assert.equal(root.style.maxHeight, '100px');
});

test('Pressing Shift+Tab on first element focuses the last menu surface element', () => {
const root = getFixture(true);
document.body.appendChild(root);
const firstItem = root.querySelectorAll(strings.FOCUSABLE_ELEMENTS)[0];
const lastItem = root.querySelectorAll(strings.FOCUSABLE_ELEMENTS)[1];
const component = new MDCMenuSurface(root);
component.open = true;

firstItem.focus();
component.getDefaultFoundation().handleKeydown({key: 'Tab', shiftKey: true, preventDefault: () => {}});
assert.equal(document.activeElement, lastItem);

component.open = false;
document.body.removeChild(root);
});

test('Pressing Tab on last element focuses the first menu surface element', () => {
const root = getFixture(true);
document.body.appendChild(root);
const firstItem = root.querySelectorAll(strings.FOCUSABLE_ELEMENTS)[0];
const lastItem = root.querySelectorAll(strings.FOCUSABLE_ELEMENTS)[1];
const component = new MDCMenuSurface(root);
component.open = true;

lastItem.focus();
component.getDefaultFoundation().handleKeydown({key: 'Tab', shiftKey: false, preventDefault: () => {}});
assert.equal(document.activeElement, firstItem);

component.open = false;
document.body.removeChild(root);
});
53 changes: 3 additions & 50 deletions test/unit/mdc-menu-surface/menu-surface.foundation.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ test('exports Corner', () => {
test('defaultAdapter returns a complete adapter implementation', () => {
verifyDefaultAdapter(MDCMenuSurfaceFoundation, [
'addClass', 'removeClass', 'hasClass', 'hasAnchor', 'notifyClose', 'notifyOpen', 'isElementInContainer',
'isRtl', 'setTransformOrigin', 'isFocused', 'saveFocus', 'restoreFocus', 'isFirstElementFocused',
'isLastElementFocused', 'focusFirstElement', 'focusLastElement', 'getInnerDimensions', 'getAnchorDimensions',
'getWindowDimensions', 'getBodyDimensions', 'getWindowScroll', 'setPosition', 'setMaxHeight',
'isRtl', 'setTransformOrigin', 'isFocused', 'saveFocus', 'restoreFocus', 'getInnerDimensions',
'getAnchorDimensions', 'getWindowDimensions', 'getBodyDimensions', 'getWindowScroll', 'setPosition',
'setMaxHeight',
]);
});

Expand Down Expand Up @@ -717,53 +717,6 @@ test('#handleKeydown with Escape key closes the menu surface and sends close eve
td.verify(mockAdapter.notifyClose());
});

test('#handleKeydown with Tab key on the last element, it moves to the first', () => {
const {foundation, mockAdapter} = setupTest();
const clock = installClock();
const target = {};
const event = {target, key: 'Tab', preventDefault: () => {}};

td.when(mockAdapter.isLastElementFocused()).thenReturn(true);

foundation.init();
foundation.handleKeydown(event);
clock.tick(numbers.SELECTED_TRIGGER_DELAY);
clock.runToFrame();
td.verify(mockAdapter.focusFirstElement());
});

test('#handleKeydown with Shift+Tab keys on the first element, it moves to the last', () => {
const {foundation, mockAdapter} = setupTest();
const clock = installClock();
const target = {};
const event = {target, key: 'Tab', shiftKey: true, preventDefault: () => {}};

td.when(mockAdapter.isFirstElementFocused()).thenReturn(true);

foundation.init();
foundation.handleKeydown(event);
clock.tick(numbers.SELECTED_TRIGGER_DELAY);
clock.runToFrame();
td.verify(mockAdapter.focusLastElement(), {times: 1});
});

test('#handleKeydown with Shift+Tab on middle element does not cause focus to wrap around to first/last', () => {
const {foundation, mockAdapter} = setupTest();
const clock = installClock();
const target = {};
const event = {target, key: 'Tab', shiftKey: true, preventDefault: () => {}};

td.when(mockAdapter.isFirstElementFocused()).thenReturn(false);
td.when(mockAdapter.isLastElementFocused()).thenReturn(false);

foundation.init();
foundation.handleKeydown(event);
clock.tick(numbers.SELECTED_TRIGGER_DELAY);
clock.runToFrame();
td.verify(mockAdapter.focusFirstElement(), {times: 0});
td.verify(mockAdapter.focusLastElement(), {times: 0});
});

test('#handleKeydown on any other key, do not prevent default on the event', () => {
const {foundation} = setupTest();
const clock = installClock();
Expand Down

0 comments on commit 99af567

Please sign in to comment.