Skip to content

Commit

Permalink
fix(popover-edit): unable to close focus content using the keyboard (#…
Browse files Browse the repository at this point in the history
…18945)

The popover edit shows some content either when a cell is hovered or when it's focused, but the problem is that keyboard users don't have a way of closing the focus content without moving focus completely out of the table, because tabbing again usually lands on a different cell. These changes allow for the focus content to be removed by pressing the Escape key.

(cherry picked from commit 17f8e94)
  • Loading branch information
crisbeto authored and jelbourn committed Mar 31, 2020
1 parent 897dee4 commit e7a197a
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 10 deletions.
15 changes: 15 additions & 0 deletions src/cdk-experimental/popover-edit/popover-edit.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,21 @@ describe('CDK Popover Edit', () => {
expect(component.hoverContentStateForRow(4)).toBe(HoverContentState.FOCUSABLE);
}));

it('should close the focus content when pressing escape', fakeAsync(() => {
expect(component.hoverContentStateForRow(2)).toBe(HoverContentState.OFF);

component.focusEditCell(2);
tick(1);

expect(component.hoverContentStateForRow(2)).toBe(HoverContentState.ON);

const event = new KeyboardEvent('keydown', {bubbles: true, key: 'Escape'});
component.getEditCell(2).dispatchEvent(event);
tick(1);

expect(component.hoverContentStateForRow(2)).toBe(HoverContentState.OFF);
}));

it('shows hover content for the editing row and makes the rows above and below ' +
'focusable unless focus is in a different table row in which case it takes priority',
fakeAsync(() => {
Expand Down
24 changes: 14 additions & 10 deletions src/cdk-experimental/popover-edit/table-directives.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,21 +104,25 @@ export class CdkEditable implements AfterViewInit, OnDestroy {

// Track focus within the table to hide/show/make focusable hover content.
fromEventPattern<FocusEvent>(
(handler) => element.addEventListener('focus', handler, true),
(handler) => element.removeEventListener('focus', handler, true)
handler => element.addEventListener('focus', handler, true),
handler => element.removeEventListener('focus', handler, true)
).pipe(
takeUntil(this.destroyed),
toClosest(ROW_SELECTOR),
share(),
).subscribe(this.editEventDispatcher.focused);
fromEventPattern<FocusEvent>(
(handler) => element.addEventListener('blur', handler, true),
(handler) => element.removeEventListener('blur', handler, true)
).pipe(
takeUntil(this.destroyed),
mapTo(null),
share(),
).subscribe(this.editEventDispatcher.focused);

merge(
fromEventPattern<FocusEvent>(
handler => element.addEventListener('blur', handler, true),
handler => element.removeEventListener('blur', handler, true)
),
fromEvent<KeyboardEvent>(element, 'keydown').pipe(filter(event => event.key === 'Escape'))
).pipe(
takeUntil(this.destroyed),
mapTo(null),
share(),
).subscribe(this.editEventDispatcher.focused);

// Keep track of rows within the table. This is used to know which rows with hover content
// are first or last in the table. They are kept focusable in case focus enters from above
Expand Down

0 comments on commit e7a197a

Please sign in to comment.