diff --git a/src/cdk-experimental/popover-edit/edit-event-dispatcher.ts b/src/cdk-experimental/popover-edit/edit-event-dispatcher.ts index acfa60e2ab4..f99f600f6cd 100644 --- a/src/cdk-experimental/popover-edit/edit-event-dispatcher.ts +++ b/src/cdk-experimental/popover-edit/edit-event-dispatcher.ts @@ -6,16 +6,41 @@ * found in the LICENSE file at https://angular.io/license */ -import {Injectable} from '@angular/core'; -import {Observable, Subject, timer} from 'rxjs'; -import {audit, distinctUntilChanged, filter, map, share} from 'rxjs/operators'; +import {Injectable, NgZone} from '@angular/core'; +import {combineLatest, MonoTypeOperatorFunction, Observable, pipe, Subject} from 'rxjs'; +import { + audit, + auditTime, + debounceTime, + distinctUntilChanged, + filter, + map, + share, + skip, + startWith, +} from 'rxjs/operators'; import {CELL_SELECTOR, ROW_SELECTOR} from './constants'; import {closest} from './polyfill'; import {EditRef} from './edit-ref'; -/** The delay between mouse out events and hiding hover content. */ -const DEFAULT_MOUSE_OUT_DELAY_MS = 30; +/** The delay applied to mouse events before hiding or showing hover content. */ +const MOUSE_EVENT_DELAY_MS = 40; + +/** The delay for reacting to focus/blur changes. */ +const FOCUS_DELAY = 0; + +/** + * The possible states for hover content: + * OFF - Not rendered. + * FOCUSABLE - Rendered in the dom and stylyed for its contents to be focusable but invisible. + * ON - Rendered and fully visible. + */ +export const enum HoverContentState { + OFF = 0, + FOCUSABLE, + ON, +} /** * Service for sharing delegated events and state for triggering table edits. @@ -28,7 +53,13 @@ export class EditEventDispatcher { /** A subject that indicates which table row is currently hovered. */ readonly hovering = new Subject(); - /** A subject that emits mouse move events for table rows. */ + /** A subject that indicates which table row currently contains focus. */ + readonly focused = new Subject(); + + /** A subject that indicates all elements in the table matching ROW_SELECTOR. */ + readonly allRows = new Subject(); + + /** A subject that emits mouse move events from the table indicating the targeted row. */ readonly mouseMove = new Subject(); /** The EditRef for the currently active edit lens (if any). */ @@ -37,13 +68,76 @@ export class EditEventDispatcher { } private _editRef: EditRef|null = null; + // Optimization: Precompute common pipeable operators used per row/cell. + private readonly _distinctUntilChanged = + distinctUntilChanged(); + private readonly _startWithNull = startWith(null); + private readonly _distinctShare = pipe( + this._distinctUntilChanged as MonoTypeOperatorFunction, + share(), + ); + private readonly _startWithNullDistinct = pipe( + this._startWithNull, + this._distinctUntilChanged as MonoTypeOperatorFunction, + ); + + /** An observable that emits the row containing focus or an active edit. */ + readonly editingOrFocused = combineLatest( + this.editing.pipe( + map(cell => closest(cell, ROW_SELECTOR)), + this._startWithNull, + ), + this.focused.pipe(this._startWithNull), + ).pipe( + map(([editingRow, focusedRow]) => focusedRow || editingRow), + this._distinctUntilChanged as MonoTypeOperatorFunction, + auditTime(FOCUS_DELAY), // Use audit to skip over blur events to the next focused element. + this._distinctUntilChanged as MonoTypeOperatorFunction, + share(), + ); + + /** Tracks rows that contain hover content with a reference count. */ + private _rowsWithHoverContent = new WeakMap(); + /** The table cell that has an active edit lens (or null). */ private _currentlyEditing: Element|null = null; - private readonly _hoveringDistinct = this.hovering.pipe(distinctUntilChanged(), share()); - private readonly _editingDistinct = this.editing.pipe(distinctUntilChanged(), share()); + /** The combined set of row hover content states organized by row. */ + private readonly _hoveredContentStateDistinct = combineLatest( + this._getFirstRowWithHoverContent(), + this._getLastRowWithHoverContent(), + this.editingOrFocused, + this.hovering.pipe( + distinctUntilChanged(), + audit(row => this.mouseMove.pipe( + filter(mouseMoveRow => row === mouseMoveRow), + this._startWithNull, + debounceTime(MOUSE_EVENT_DELAY_MS)), + ), + this._startWithNullDistinct, + ), + ).pipe( + skip(1), // Skip the initial emission of [null, null, null, null]. + map(computeHoverContentState), + distinctUntilChanged(areMapEntriesEqual), + // Optimization: Enter the zone before share() so that we trigger a single + // ApplicationRef.tick for all row updates. + this._enterZone(), + share(), + ); - constructor() { + private readonly _editingDistinct = this.editing.pipe( + distinctUntilChanged(), + this._enterZone(), + share(), + ); + + // Optimization: Share row events observable with subsequent callers. + // At startup, calls will be sequential by row. + private _lastSeenRow: Element|null = null; + private _lastSeenRowHoverOrFocus: Observable|null = null; + + constructor(private readonly _ngZone: NgZone) { this._editingDistinct.subscribe(cell => { this._currentlyEditing = cell; }); @@ -58,7 +152,7 @@ export class EditEventDispatcher { return this._editingDistinct.pipe( map(editCell => editCell === (cell || (cell = closest(element, CELL_SELECTOR)))), - distinctUntilChanged(), + this._distinctUntilChanged as MonoTypeOperatorFunction, ); } @@ -88,20 +182,123 @@ export class EditEventDispatcher { this._editRef = null; } + /** Adds the specified table row to be tracked for first/last row comparisons. */ + registerRowWithHoverContent(row: Element): void { + this._rowsWithHoverContent.set(row, (this._rowsWithHoverContent.get(row) || 0) + 1); + } + + /** + * Reference decrements and ultimately removes the specified table row from first/last row + * comparisons. + */ + deregisterRowWithHoverContent(row: Element): void { + const refCount = this._rowsWithHoverContent.get(row) || 0; + + if (refCount <= 1) { + this._rowsWithHoverContent.delete(row); + } else { + this._rowsWithHoverContent.set(row, refCount - 1); + } + } + /** * Gets an Observable that emits true when the specified element's row - * is being hovered over and false when not. Hovering is defined as when - * the mouse has momentarily stopped moving over the cell. + * contains the focused element or is being hovered over and false when not. + * Hovering is defined as when the mouse has momentarily stopped moving over the cell. */ - hoveringOnRow(element: Element|EventTarget): Observable { - let row: Element|null = null; + hoverOrFocusOnRow(row: Element): Observable { + if (row !== this._lastSeenRow) { + this._lastSeenRow = row; + this._lastSeenRowHoverOrFocus = this._hoveredContentStateDistinct.pipe( + map(state => state.get(row) || HoverContentState.OFF), + this._distinctShare, + ); + } - return this._hoveringDistinct.pipe( - map(hoveredRow => hoveredRow === (row || (row = closest(element, ROW_SELECTOR)))), - audit( - (hovering) => hovering ? this.mouseMove.pipe(filter(hoveredRow => hoveredRow === row)) : - timer(DEFAULT_MOUSE_OUT_DELAY_MS)), - distinctUntilChanged(), + return this._lastSeenRowHoverOrFocus!; + } + + /** + * RxJS operator that enters the Angular zone, used to reduce boilerplate in + * re-entering the zone for stream pipelines. + */ + private _enterZone(): MonoTypeOperatorFunction { + return (source: Observable) => + new Observable((observer) => source.subscribe({ + next: (value) => this._ngZone.run(() => observer.next(value)), + error: (err) => observer.error(err), + complete: () => observer.complete() + })); + } + + private _getFirstRowWithHoverContent(): Observable { + return this._mapAllRowsToSingleRow(rows => { + for (let i = 0, row; row = rows[i]; i++) { + if (this._rowsWithHoverContent.has(row as Element)) { + return row as Element; + } + } + return null; + }); + } + + private _getLastRowWithHoverContent(): Observable { + return this._mapAllRowsToSingleRow(rows => { + for (let i = rows.length - 1, row; row = rows[i]; i--) { + if (this._rowsWithHoverContent.has(row as Element)) { + return row as Element; + } + } + return null; + }); + } + + private _mapAllRowsToSingleRow(mapper: (rows: NodeList) => Element|null): + Observable { + return this.allRows.pipe( + map(mapper), + this._startWithNullDistinct, ); } } + +function computeHoverContentState([firstRow, lastRow, activeRow, hoverRow]: Array): + Map { + const hoverContentState = new Map(); + + // Add focusable rows. + for (const focussableRow of [ + firstRow, + lastRow, + activeRow && activeRow.previousElementSibling, + activeRow && activeRow.nextElementSibling, + ]) { + if (focussableRow) { + hoverContentState.set(focussableRow as Element, HoverContentState.FOCUSABLE); + } + } + + // Add/overwrite with fully visible rows. + for (const onRow of [activeRow, hoverRow]) { + if (onRow) { + hoverContentState.set(onRow, HoverContentState.ON); + } + } + + return hoverContentState; +} + +function areMapEntriesEqual(a: Map, b: Map): boolean { + if (a.size !== b.size) { + return false; + } + + // TODO: use Map.prototype.entries once we're off IE11. + for (const aKey of Array.from(a.keys())) { + if (b.get(aKey) !== a.get(aKey)) { + return false; + } + } + + return true; +} diff --git a/src/cdk-experimental/popover-edit/polyfill.ts b/src/cdk-experimental/popover-edit/polyfill.ts index 4a6ea1f21f0..980b4fb89b1 100644 --- a/src/cdk-experimental/popover-edit/polyfill.ts +++ b/src/cdk-experimental/popover-edit/polyfill.ts @@ -13,11 +13,22 @@ export function matches(element: Element, selector: string): boolean { (element as any)['msMatchesSelector'](selector); } -/** IE 11 compatible closest implementation. */ -export function closest(element: EventTarget|Element|null|undefined, selector: string): Element| - null { +/** IE 11 compatible closest implementation that is able to start from non-Element Nodes. */ +export function closest(element: EventTarget|Element|null|undefined, selector: string): + Element|null { if (!(element instanceof Node)) { return null; } + let curr: Node|null = element; + while (curr != null && !(curr instanceof Element)) { + curr = curr.parentNode; + } + + return curr && (hasNativeClosest ? + curr.closest(selector) : polyfillClosest(curr, selector)) as Element|null; +} + +/** Polyfill for browsers without Element.closest. */ +function polyfillClosest(element: Element, selector: string): Element|null { let curr: Node|null = element; while (curr != null && !(curr instanceof Element && matches(curr, selector))) { curr = curr.parentNode; @@ -25,3 +36,5 @@ export function closest(element: EventTarget|Element|null|undefined, selector: s return (curr || null) as Element|null; } + +const hasNativeClosest = !!Element.prototype.closest; diff --git a/src/cdk-experimental/popover-edit/popover-edit.spec.ts b/src/cdk-experimental/popover-edit/popover-edit.spec.ts index 92531a1f098..876a28381ce 100644 --- a/src/cdk-experimental/popover-edit/popover-edit.spec.ts +++ b/src/cdk-experimental/popover-edit/popover-edit.spec.ts @@ -10,7 +10,12 @@ import {BidiModule, Direction} from '@angular/cdk/bidi'; import {OverlayContainer} from '@angular/cdk/overlay'; import {BehaviorSubject} from 'rxjs'; -import {CdkPopoverEditColspan, CdkPopoverEditModule, PopoverEditClickOutBehavior} from './index'; +import { + CdkPopoverEditColspan, + CdkPopoverEditModule, + HoverContentState, + PopoverEditClickOutBehavior, +} from './index'; const NAME_EDIT_TEMPLATE = `
@@ -76,7 +81,8 @@ abstract class BaseTestComponent { row.dispatchEvent(new Event('mouseover', {bubbles: true})); row.dispatchEvent(new Event('mousemove', {bubbles: true})); - tick(31); + // Wait for the mouse hover debounce in edit-event-dispatcher. + tick(41); } getRows() { @@ -92,6 +98,16 @@ abstract class BaseTestComponent { this.getEditCell(rowIndex, cellIndex).focus(); } + hoverContentStateForRow(rowIndex = 0) { + const openButton = this.getOpenButton(rowIndex); + + if (!openButton) { + return HoverContentState.OFF; + } + return parseInt(getComputedStyle(openButton.parentNode as Element).opacity || '', 10) === 0 ? + HoverContentState.FOCUSABLE : HoverContentState.ON; + } + getOpenButton(rowIndex = 0) { return this.getEditCell(rowIndex).querySelector('.open') as HTMLElement|null; } @@ -325,7 +341,7 @@ describe('CDK Popover Edit', () => { let fixture: ComponentFixture; let overlayContainer: OverlayContainer; - beforeEach(() => { + beforeEach(fakeAsync(() => { TestBed.configureTestingModule({ imports: [CdkTableModule, CdkPopoverEditModule, CommonModule, FormsModule, BidiModule], declarations: [componentClass], @@ -336,7 +352,8 @@ describe('CDK Popover Edit', () => { fixture = TestBed.createComponent(componentClass); component = fixture.componentInstance; fixture.detectChanges(); - }); + tick(10); + })); afterEach(() => { // The overlay container's `ngOnDestroy` won't be called between test runs so we need @@ -345,41 +362,107 @@ describe('CDK Popover Edit', () => { overlayContainer.ngOnDestroy(); }); + describe('row hover content', () => { + it('makes the first and last rows focusable but invisible', fakeAsync(() => { + const rows = component.getRows(); + + expect(component.hoverContentStateForRow(0)).toBe(HoverContentState.FOCUSABLE); + expect(component.hoverContentStateForRow(rows.length - 1)) + .toBe(HoverContentState.FOCUSABLE); + })); + }); + describe('triggering edit', () => { it('shows and hides on-hover content only after a delay', fakeAsync(() => { - const [row0, row1] = component.getRows(); - row0.dispatchEvent(new Event('mouseover', {bubbles: true})); - row0.dispatchEvent(new Event('mousemove', {bubbles: true})); + const [row0, row1] = component.getRows(); + row0.dispatchEvent(new Event('mouseover', {bubbles: true})); + row0.dispatchEvent(new Event('mousemove', {bubbles: true})); - expect(component.getOpenButton(0)).toBe(null); + expect(component.hoverContentStateForRow(0)).toBe(HoverContentState.FOCUSABLE); - tick(20); - row0.dispatchEvent(new Event('mousemove', {bubbles: true})); - tick(20); + tick(20); + row0.dispatchEvent(new Event('mousemove', {bubbles: true})); + tick(20); - expect(component.getOpenButton(0)).toBe(null); + expect(component.hoverContentStateForRow(0)).toBe(HoverContentState.FOCUSABLE); - tick(11); + tick(31); - expect(component.getOpenButton(0)).toEqual(jasmine.any(HTMLElement)); + expect(component.hoverContentStateForRow(0)).toBe(HoverContentState.ON); - row1.dispatchEvent(new Event('mouseover', {bubbles: true})); - row1.dispatchEvent(new Event('mousemove', {bubbles: true})); + row1.dispatchEvent(new Event('mouseover', {bubbles: true})); + row1.dispatchEvent(new Event('mousemove', {bubbles: true})); - expect(component.getOpenButton(0)).toEqual(jasmine.any(HTMLElement)); - expect(component.getOpenButton(1)).toBe(null); + expect(component.hoverContentStateForRow(0)).toBe(HoverContentState.ON); + expect(component.hoverContentStateForRow(1)).toBe(HoverContentState.OFF); - tick(31); + tick(41); - expect(component.getOpenButton(0)).toBe(null); - expect(component.getOpenButton(1)).toEqual(jasmine.any(HTMLElement)); - })); + expect(component.hoverContentStateForRow(0)).toBe(HoverContentState.FOCUSABLE); + expect(component.hoverContentStateForRow(1)).toBe(HoverContentState.ON); + })); + it('shows hover content for the focused row and makes the rows above and below focusable', + fakeAsync(() => { + expect(component.hoverContentStateForRow(1)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(2)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(3)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(4)).toBe(HoverContentState.FOCUSABLE); + + component.focusEditCell(2); + tick(1); + + expect(component.hoverContentStateForRow(2)).toBe(HoverContentState.ON); + expect(component.hoverContentStateForRow(1)).toBe(HoverContentState.FOCUSABLE); + expect(component.hoverContentStateForRow(3)).toBe(HoverContentState.FOCUSABLE); + + component.focusEditCell(4); + tick(1); + + expect(component.hoverContentStateForRow(2)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(4)).toBe(HoverContentState.ON); + expect(component.hoverContentStateForRow(3)).toBe(HoverContentState.FOCUSABLE); + + component.getEditCell(4).blur(); + tick(1); + + expect(component.hoverContentStateForRow(1)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(2)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(3)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(4)).toBe(HoverContentState.FOCUSABLE); + })); + + 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(() => { + expect(component.hoverContentStateForRow(1)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(2)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(3)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(4)).toBe(HoverContentState.FOCUSABLE); + + component.openLens(2); + tick(1); + + expect(component.hoverContentStateForRow(2)).toBe(HoverContentState.ON); + expect(component.hoverContentStateForRow(1)).toBe(HoverContentState.FOCUSABLE); + expect(component.hoverContentStateForRow(3)).toBe(HoverContentState.FOCUSABLE); + + component.focusEditCell(4); + tick(1); + + expect(component.hoverContentStateForRow(2)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(4)).toBe(HoverContentState.ON); + expect(component.hoverContentStateForRow(3)).toBe(HoverContentState.FOCUSABLE); + })); + }); + + describe('triggering edit', () => { it('opens edit from on-hover button', fakeAsync(() => { component.triggerHoverState(); component.clickOpenButton(); expect(component.lensIsOpen()).toBe(true); + clearLeftoverTimers(); })); it('opens edit from Enter on focued cell', fakeAsync(() => { @@ -387,6 +470,7 @@ describe('CDK Popover Edit', () => { component.openLens(); expect(component.lensIsOpen()).toBe(true); + clearLeftoverTimers(); })); }); @@ -475,6 +559,7 @@ describe('CDK Popover Edit', () => { // Focus should have moved to the top of the lens. expect(document.activeElement).toBe(focusableElements[1]); expect(component.lensIsOpen()).toBe(true); + clearLeftoverTimers(); })); it('moves focus to the next cell when focus leaves end of lens with cdkPopoverEditTabOut', @@ -491,6 +576,7 @@ describe('CDK Popover Edit', () => { // Focus should have moved to the next editable cell. expect(document.activeElement).toBe(component.getEditCell(1, 1)); expect(component.lensIsOpen()).toBe(false); + clearLeftoverTimers(); })); it(`moves focus to the previous cell when focus leaves end of lens with @@ -507,6 +593,7 @@ cdkPopoverEditTabOut`, fakeAsync(() => { // Focus should have moved to the next editable cell. expect(document.activeElement).toBe(component.getEditCell(0, 1)); expect(component.lensIsOpen()).toBe(false); + clearLeftoverTimers(); })); }); }); @@ -516,6 +603,7 @@ cdkPopoverEditTabOut`, fakeAsync(() => { component.openLens(); expect(component.getInput()!.value).toBe('Hydrogen'); + clearLeftoverTimers(); })); it('positions the lens at the top left corner and spans the full width of the cell', @@ -528,6 +616,7 @@ cdkPopoverEditTabOut`, fakeAsync(() => { expect(paneRect.width).toBe(cellRect.width); expect(paneRect.left).toBe(cellRect.left); expect(paneRect.top).toBe(cellRect.top); + clearLeftoverTimers(); })); it('adjusts the positioning of the lens based on colspan', fakeAsync(() => { @@ -559,6 +648,7 @@ cdkPopoverEditTabOut`, fakeAsync(() => { expect(paneRect.top).toBe(cellRects[0].top); expect(paneRect.left).toBe(cellRects[0].left); expect(paneRect.right).toBe(cellRects[2].right); + clearLeftoverTimers(); })); it('updates the form and submits, closing the lens', fakeAsync(() => { @@ -569,9 +659,9 @@ cdkPopoverEditTabOut`, fakeAsync(() => { component.clickSubmitButton(); fixture.detectChanges(); - - expect(component.getEditCell().textContent!.trim()).toBe('Hydragon'); + expect(component.getEditCell().firstChild!.textContent!.trim()).toBe('Hydragon'); expect(component.lensIsOpen()).toBe(false); + clearLeftoverTimers(); })); it('does not close the lens on submit when form is invalid', fakeAsync(() => { @@ -583,6 +673,7 @@ cdkPopoverEditTabOut`, fakeAsync(() => { component.clickSubmitButton(); expect(component.lensIsOpen()).toBe(true); + clearLeftoverTimers(); })); it('closes lens on submit when form is invalid with ' + @@ -596,6 +687,7 @@ cdkPopoverEditTabOut`, fakeAsync(() => { component.clickSubmitButton(); expect(component.lensIsOpen()).toBe(false); + clearLeftoverTimers(); })); it('closes the lens on close', fakeAsync(() => { @@ -604,6 +696,7 @@ cdkPopoverEditTabOut`, fakeAsync(() => { component.clickCloseButton(); expect(component.lensIsOpen()).toBe(false); + clearLeftoverTimers(); })); it('closes and reopens a lens with modified value persisted', fakeAsync(() => { @@ -615,12 +708,13 @@ cdkPopoverEditTabOut`, fakeAsync(() => { component.clickCloseButton(); fixture.detectChanges(); - expect(component.getEditCell().textContent!.trim()).toBe('Hydrogen'); + expect(component.getEditCell().firstChild!.textContent!.trim()).toBe('Hydrogen'); expect(component.lensIsOpen()).toBe(false); component.openLens(); expect(component.getInput()!.value).toBe('Hydragon'); + clearLeftoverTimers(); })); it('resets the lens to original value', fakeAsync(() => { @@ -632,6 +726,7 @@ cdkPopoverEditTabOut`, fakeAsync(() => { component.clickRevertButton(); expect(component.getInput()!.value).toBe('Hydrogen'); + clearLeftoverTimers(); })); it('resets the lens to previously submitted value', fakeAsync(() => { @@ -651,6 +746,7 @@ cdkPopoverEditTabOut`, fakeAsync(() => { component.clickRevertButton(); expect(component.getInput()!.value).toBe('Hydragon'); + clearLeftoverTimers(); })); it('closes the lens on escape', fakeAsync(() => { @@ -660,6 +756,7 @@ cdkPopoverEditTabOut`, fakeAsync(() => { new KeyboardEvent('keyup', {bubbles: true, key: 'Escape'})); expect(component.lensIsOpen()).toBe(false); + clearLeftoverTimers(); })); it('does not close the lens on click within lens', fakeAsync(() => { @@ -668,6 +765,7 @@ cdkPopoverEditTabOut`, fakeAsync(() => { component.getInput()!.dispatchEvent(new Event('click', {bubbles: true})); expect(component.lensIsOpen()).toBe(true); + clearLeftoverTimers(); })); it('closes the lens on outside click', fakeAsync(() => { @@ -679,8 +777,8 @@ cdkPopoverEditTabOut`, fakeAsync(() => { fixture.detectChanges(); expect(component.lensIsOpen()).toBe(false); - expect(component.getEditCell().textContent!.trim()).toBe('Hydrogen'); - + expect(component.getEditCell().firstChild!.textContent!.trim()).toBe('Hydrogen'); + clearLeftoverTimers(); })); it('submits the lens on outside click with ' + @@ -694,7 +792,8 @@ cdkPopoverEditTabOut`, fakeAsync(() => { fixture.detectChanges(); expect(component.lensIsOpen()).toBe(false); - expect(component.getEditCell().textContent!.trim()).toBe('Hydragon'); + expect(component.getEditCell().firstChild!.textContent!.trim()).toBe('Hydragon'); + clearLeftoverTimers(); })); it('does nothing on outside click with ' + @@ -708,13 +807,15 @@ cdkPopoverEditTabOut`, fakeAsync(() => { fixture.detectChanges(); expect(component.lensIsOpen()).toBe(true); - expect(component.getEditCell().textContent!.trim()).toBe('Hydrogen'); + expect(component.getEditCell().firstChild!.textContent!.trim()).toBe('Hydrogen'); + clearLeftoverTimers(); })); it('sets focus on the first input in the lens', fakeAsync(() => { component.openLens(); expect(document.activeElement).toBe(component.getInput()); + clearLeftoverTimers(); })); it('returns focus to the edited cell after closing', fakeAsync(() => { @@ -723,6 +824,7 @@ cdkPopoverEditTabOut`, fakeAsync(() => { component.clickCloseButton(); expect(document.activeElement).toBe(component.getEditCell()); + clearLeftoverTimers(); })); it('does not focus to the edited cell after closing if another element ' + @@ -733,6 +835,7 @@ cdkPopoverEditTabOut`, fakeAsync(() => { component.getEditCell(1).dispatchEvent(new Event('click', {bubbles: true})); expect(document.activeElement).toBe(component.getEditCell(1)); + clearLeftoverTimers(); })); it('should pass the directionality to the overlay', fakeAsync(() => { @@ -742,10 +845,10 @@ cdkPopoverEditTabOut`, fakeAsync(() => { component.openLens(); expect(component.getEditBoundingBox()!.getAttribute('dir')).toBe('rtl'); + clearLeftoverTimers(); })); - }); - }); + }); } }); @@ -774,3 +877,10 @@ function getCells(row: Element): HTMLElement[] { return getElements(row, '.cdk-cell, td'); } + +// Common actions like mouse events and focus/blur cause timers to be fired off. +// When not testing this behavior directly, use this function to clear any timers that were +// created in passing. +function clearLeftoverTimers() { + tick(100); +} diff --git a/src/cdk-experimental/popover-edit/public-api.ts b/src/cdk-experimental/popover-edit/public-api.ts index 3dfcff15503..492487e7229 100644 --- a/src/cdk-experimental/popover-edit/public-api.ts +++ b/src/cdk-experimental/popover-edit/public-api.ts @@ -13,3 +13,6 @@ export * from './lens-directives'; export * from './popover-edit-module'; export * from './popover-edit-position-strategy-factory'; export * from './table-directives'; + +export {CELL_SELECTOR as _CELL_SELECTOR} from './constants'; +export {closest as _closest} from './polyfill'; diff --git a/src/cdk-experimental/popover-edit/table-directives.ts b/src/cdk-experimental/popover-edit/table-directives.ts index 2eda6df3659..bb3d6aa07fc 100644 --- a/src/cdk-experimental/popover-edit/table-directives.ts +++ b/src/cdk-experimental/popover-edit/table-directives.ts @@ -19,11 +19,20 @@ import { ViewContainerRef, HostListener, } from '@angular/core'; -import {fromEvent, merge, ReplaySubject} from 'rxjs'; -import {debounceTime, filter, map, mapTo, startWith, takeUntil} from 'rxjs/operators'; +import {fromEvent, fromEventPattern, merge, ReplaySubject} from 'rxjs'; +import { + filter, + map, + mapTo, + share, + startWith, + takeUntil, + throttleTime, + withLatestFrom, +} from 'rxjs/operators'; import {CELL_SELECTOR, EDIT_PANE_CLASS, EDIT_PANE_SELECTOR, ROW_SELECTOR} from './constants'; -import {EditEventDispatcher} from './edit-event-dispatcher'; +import {EditEventDispatcher, HoverContentState} from './edit-event-dispatcher'; import {EditServices} from './edit-services'; import {FocusDispatcher} from './focus-dispatcher'; import { @@ -43,11 +52,8 @@ export interface CdkPopoverEditColspan { after?: number; } -/** - * The delay between the mouse entering a row and the mouse stopping its movement before - * showing on-hover content. - */ -const DEFAULT_MOUSE_MOVE_DELAY_MS = 30; +/** Used for rate-limiting mousemove events. */ +const MOUSE_MOVE_THROTTLE_TIME_MS = 10; /** * A directive that must be attached to enable editability on a table. @@ -82,6 +88,7 @@ export class CdkEditable implements AfterViewInit, OnDestroy { map((event: UIEvent) => closest(event.target, selector)); this.ngZone.runOutsideAngular(() => { + // Track mouse movement over the table to hide/show hover content. fromEvent(element, 'mouseover').pipe( takeUntil(this.destroyed), toClosest(ROW_SELECTOR), @@ -92,10 +99,41 @@ export class CdkEditable implements AfterViewInit, OnDestroy { ).subscribe(this.editEventDispatcher.hovering); fromEvent(element, 'mousemove').pipe( takeUntil(this.destroyed), - debounceTime(DEFAULT_MOUSE_MOVE_DELAY_MS), + throttleTime(MOUSE_MOVE_THROTTLE_TIME_MS), toClosest(ROW_SELECTOR), ).subscribe(this.editEventDispatcher.mouseMove); + // Track focus within the table to hide/show/make focusable hover content. + fromEventPattern( + (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( + (handler) => element.addEventListener('blur', handler, true), + (handler) => element.removeEventListener('blur', handler, true) + ).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 + // or below the table. + this.ngZone.onStable.pipe( + takeUntil(this.destroyed), + // Optimization: ignore dom changes while focus is within the table as we already + // ensure that rows above and below the focused/active row are tabbable. + withLatestFrom(this.editEventDispatcher.editingOrFocused), + filter(([_, activeRow]) => activeRow == null), + map(() => element.querySelectorAll(ROW_SELECTOR)), + share(), + ).subscribe(this.editEventDispatcher.allRows); + fromEvent(element, 'keyup').pipe( takeUntil(this.destroyed), filter(event => event.key === 'Enter'), @@ -200,19 +238,17 @@ export class CdkPopoverEdit implements AfterViewInit, OnDestroy { this.services.editEventDispatcher.editingCell(this.elementRef.nativeElement!) .pipe(takeUntil(this.destroyed)) .subscribe((open) => { - this.services.ngZone.run(() => { - if (open && this.template) { - if (!this.overlayRef) { - this._createEditOverlay(); - } - - this._showEditOverlay(); - } else if (this.overlayRef) { - this._maybeReturnFocusToCell(); - - this.overlayRef.detach(); + if (open && this.template) { + if (!this.overlayRef) { + this._createEditOverlay(); } - }); + + this._showEditOverlay(); + } else if (this.overlayRef) { + this._maybeReturnFocusToCell(); + + this.overlayRef.detach(); + } }); } @@ -236,7 +272,7 @@ export class CdkPopoverEdit implements AfterViewInit, OnDestroy { this.template!, this.viewContainerRef, {$implicit: this.context})); - this.focusTrap!.focusInitialElementWhenReady(); + this.focusTrap!.focusInitialElement(); // Update the size of the popup initially and on subsequent changes to // scroll position and viewport size. @@ -321,16 +357,7 @@ export class CdkPopoverEditTabOut extends CdkPopoverEdit { /** * A structural directive that shows its contents when the table row containing - * it is hovered. - * - * Note that the contents of this directive are invisible to screen readers. - * Typically this is used to show a button that launches the edit popup, which - * is ok because screen reader users can trigger edit by pressing Enter on a focused - * table cell. - * - * If this directive contains buttons for functionality other than opening edit then - * care should be taken to make sure that this functionality is also exposed in - * an accessible way. + * it is hovered or when an element in the row has focus. */ @Directive({ selector: '[cdkRowHoverContent]', @@ -339,13 +366,18 @@ export class CdkRowHoverContent implements AfterViewInit, OnDestroy { protected readonly destroyed = new ReplaySubject(); protected viewRef: EmbeddedViewRef|null = null; + private _row?: Element; + constructor( protected readonly services: EditServices, protected readonly elementRef: ElementRef, protected readonly templateRef: TemplateRef, protected readonly viewContainerRef: ViewContainerRef) {} ngAfterViewInit(): void { - this._listenForHoverEvents(); + this._row = closest(this.elementRef.nativeElement!, ROW_SELECTOR)!; + + this.services.editEventDispatcher.registerRowWithHoverContent(this._row); + this._listenForHoverAndFocusEvents(); } ngOnDestroy(): void { @@ -355,35 +387,59 @@ export class CdkRowHoverContent implements AfterViewInit, OnDestroy { if (this.viewRef) { this.viewRef.destroy(); } + + if (this._row) { + this.services.editEventDispatcher.deregisterRowWithHoverContent(this._row); + } } - protected initElement(element: HTMLElement): void; - protected initElement(): void { + /** + * Called immediately after the hover content is created and added to the dom. + * In the CDK version, this is a noop but subclasses such as MatRowHoverContent use this + * to prepare/style the inserted element. + */ + protected initElement(_: HTMLElement): void { } - protected prepareElement(element: HTMLElement): void; - protected prepareElement(): void { + /** + * Called when the hover content needs to be focusable to preserve a reasonable tab ordering + * but should not yet be shown. + */ + protected makeElementHiddenButFocusable(element: HTMLElement): void { + element.style.opacity = '0'; } - private _listenForHoverEvents(): void { - this.services.editEventDispatcher.hoveringOnRow(this.elementRef.nativeElement!) + /** + * Called when the hover content needs to be focusable to preserve a reasonable tab ordering + * but should not yet be shown. + */ + protected makeElementVisible(element: HTMLElement): void { + element.style.opacity = ''; + } + + private _listenForHoverAndFocusEvents(): void { + this.services.editEventDispatcher.hoverOrFocusOnRow(this._row!) .pipe(takeUntil(this.destroyed)) - .subscribe(isHovering => { - this.services.ngZone.run(() => { - if (isHovering) { - if (!this.viewRef) { - // Not doing any positioning in CDK version. Material version - // will absolutely position on right edge of cell. - this.viewRef = this.viewContainerRef.createEmbeddedView(this.templateRef, {}); - this.initElement(this.viewRef.rootNodes[0] as HTMLElement); - } else { - this.viewContainerRef.insert(this.viewRef); - } - this.prepareElement(this.viewRef.rootNodes[0] as HTMLElement); - } else if (this.viewRef) { - this.viewContainerRef.detach(this.viewContainerRef.indexOf(this.viewRef)); + .subscribe(eventState => { + // When in FOCUSABLE state, add the hover content to the dom but make it transparent so + // that it is in the tab order relative to the currently focused row. + + if (eventState === HoverContentState.ON || eventState === HoverContentState.FOCUSABLE) { + if (!this.viewRef) { + this.viewRef = this.viewContainerRef.createEmbeddedView(this.templateRef, {}); + this.initElement(this.viewRef.rootNodes[0] as HTMLElement); + } else if (this.viewContainerRef.indexOf(this.viewRef) === -1) { + this.viewContainerRef.insert(this.viewRef!); } - }); + + if (eventState === HoverContentState.ON) { + this.makeElementVisible(this.viewRef.rootNodes[0] as HTMLElement); + } else { + this.makeElementHiddenButFocusable(this.viewRef.rootNodes[0] as HTMLElement); + } + } else if (this.viewRef) { + this.viewContainerRef.detach(this.viewContainerRef.indexOf(this.viewRef)); + } }); } } diff --git a/src/dev-app/popover-edit/assets/edit-icon.svg b/src/dev-app/popover-edit/assets/edit-icon.svg deleted file mode 100644 index 5b54e693b9e..00000000000 --- a/src/dev-app/popover-edit/assets/edit-icon.svg +++ /dev/null @@ -1 +0,0 @@ - \ No newline at end of file diff --git a/src/material-examples/popover-edit-cell-span-mat-table/popover-edit-cell-span-mat-table-example.ts b/src/material-examples/popover-edit-cell-span-mat-table/popover-edit-cell-span-mat-table-example.ts index d94bed51662..8b5ff3b6da1 100644 --- a/src/material-examples/popover-edit-cell-span-mat-table/popover-edit-cell-span-mat-table-example.ts +++ b/src/material-examples/popover-edit-cell-span-mat-table/popover-edit-cell-span-mat-table-example.ts @@ -1,8 +1,6 @@ import {Component} from '@angular/core'; import {DataSource} from '@angular/cdk/collections'; -import {DomSanitizer} from '@angular/platform-browser'; import {NgForm} from '@angular/forms'; -import {MatIconRegistry} from '@angular/material/icon'; import {BehaviorSubject, Observable} from 'rxjs'; export interface Person { @@ -38,12 +36,6 @@ export class PopoverEditCellSpanMatTableExample { readonly preservedValues = new WeakMap(); - constructor(iconRegistry: MatIconRegistry, sanitizer: DomSanitizer) { - iconRegistry.addSvgIcon( - 'edit', - sanitizer.bypassSecurityTrustResourceUrl('assets/img/examples/edit-icon.svg')); - } - onSubmit(person: Person, f: NgForm) { if (!f.valid) { return; diff --git a/src/material-examples/popover-edit-mat-table-flex/popover-edit-mat-table-flex-example.ts b/src/material-examples/popover-edit-mat-table-flex/popover-edit-mat-table-flex-example.ts index 9b64f7fdaf4..2518d346331 100644 --- a/src/material-examples/popover-edit-mat-table-flex/popover-edit-mat-table-flex-example.ts +++ b/src/material-examples/popover-edit-mat-table-flex/popover-edit-mat-table-flex-example.ts @@ -1,8 +1,6 @@ import {Component} from '@angular/core'; import {DataSource} from '@angular/cdk/collections'; -import {DomSanitizer} from '@angular/platform-browser'; import {NgForm} from '@angular/forms'; -import {MatIconRegistry} from '@angular/material/icon'; import {BehaviorSubject, Observable} from 'rxjs'; export interface PeriodicElement { @@ -50,12 +48,6 @@ export class PopoverEditMatTableFlexExample { readonly preservedNameValues = new WeakMap(); readonly preservedWeightValues = new WeakMap(); - constructor(iconRegistry: MatIconRegistry, sanitizer: DomSanitizer) { - iconRegistry.addSvgIcon( - 'edit', - sanitizer.bypassSecurityTrustResourceUrl('assets/img/examples/edit-icon.svg')); - } - onSubmitName(element: PeriodicElement, f: NgForm) { if (!f.valid) { return; } diff --git a/src/material-examples/popover-edit-mat-table/popover-edit-mat-table-example.html b/src/material-examples/popover-edit-mat-table/popover-edit-mat-table-example.html index 8177e64c564..7e0aea445b5 100644 --- a/src/material-examples/popover-edit-mat-table/popover-edit-mat-table-example.html +++ b/src/material-examples/popover-edit-mat-table/popover-edit-mat-table-example.html @@ -22,7 +22,19 @@ No. - {{element.position}} + + {{element.position}} + + + + + + + diff --git a/src/material-examples/popover-edit-mat-table/popover-edit-mat-table-example.ts b/src/material-examples/popover-edit-mat-table/popover-edit-mat-table-example.ts index de9e7f319a4..a198efff70f 100644 --- a/src/material-examples/popover-edit-mat-table/popover-edit-mat-table-example.ts +++ b/src/material-examples/popover-edit-mat-table/popover-edit-mat-table-example.ts @@ -1,8 +1,7 @@ import {Component} from '@angular/core'; import {DataSource} from '@angular/cdk/collections'; -import {DomSanitizer} from '@angular/platform-browser'; import {NgForm} from '@angular/forms'; -import {MatIconRegistry} from '@angular/material/icon'; +import {MatSnackBar} from '@angular/material/snack-bar'; import {BehaviorSubject, Observable} from 'rxjs'; export interface PeriodicElement { @@ -50,11 +49,7 @@ export class PopoverEditMatTableExample { readonly preservedNameValues = new WeakMap(); readonly preservedWeightValues = new WeakMap(); - constructor(iconRegistry: MatIconRegistry, sanitizer: DomSanitizer) { - iconRegistry.addSvgIcon( - 'edit', - sanitizer.bypassSecurityTrustResourceUrl('assets/img/examples/edit-icon.svg')); - } + constructor(private readonly _snackBar: MatSnackBar) {} onSubmitName(element: PeriodicElement, f: NgForm) { if (!f.valid) { return; } @@ -67,6 +62,15 @@ export class PopoverEditMatTableExample { element.weight = f.value.weight; } + + goodJob(element: PeriodicElement) { + this._snackBar.open(`Way to go, ${element.name}!`, undefined, {duration: 2000}); + } + + badJob(element: PeriodicElement) { + this._snackBar.open(`You have failed me for the last time, #${element.position}.`, undefined, + {duration: 2000}); + } } /** diff --git a/src/material-examples/popover-edit-tab-out-mat-table/popover-edit-tab-out-mat-table-example.ts b/src/material-examples/popover-edit-tab-out-mat-table/popover-edit-tab-out-mat-table-example.ts index 9aac77699d6..43212fe5346 100644 --- a/src/material-examples/popover-edit-tab-out-mat-table/popover-edit-tab-out-mat-table-example.ts +++ b/src/material-examples/popover-edit-tab-out-mat-table/popover-edit-tab-out-mat-table-example.ts @@ -1,8 +1,6 @@ import {Component} from '@angular/core'; import {DataSource} from '@angular/cdk/collections'; -import {DomSanitizer} from '@angular/platform-browser'; import {NgForm} from '@angular/forms'; -import {MatIconRegistry} from '@angular/material/icon'; import {BehaviorSubject, Observable} from 'rxjs'; export interface PeriodicElement { @@ -50,12 +48,6 @@ export class PopoverEditTabOutMatTableExample { readonly preservedNameValues = new WeakMap(); readonly preservedWeightValues = new WeakMap(); - constructor(iconRegistry: MatIconRegistry, sanitizer: DomSanitizer) { - iconRegistry.addSvgIcon( - 'edit', - sanitizer.bypassSecurityTrustResourceUrl('assets/img/examples/edit-icon.svg')); - } - onSubmitName(element: PeriodicElement, f: NgForm) { if (!f.valid) { return; } diff --git a/src/material-experimental/popover-edit/_popover-edit.scss b/src/material-experimental/popover-edit/_popover-edit.scss index acfdc2e1122..c84f71eecce 100644 --- a/src/material-experimental/popover-edit/_popover-edit.scss +++ b/src/material-experimental/popover-edit/_popover-edit.scss @@ -15,6 +15,10 @@ $background-color: mat-color($background, 'card'); + .mat-row-hover-content-host-cell { + position: relative; + } + .mat-row-hover-content { align-items: center; background: mat-edit-hover-content-background(90deg, $background-color); diff --git a/src/material-experimental/popover-edit/popover-edit.spec.ts b/src/material-experimental/popover-edit/popover-edit.spec.ts index 082bcb80244..8283dafc93f 100644 --- a/src/material-experimental/popover-edit/popover-edit.spec.ts +++ b/src/material-experimental/popover-edit/popover-edit.spec.ts @@ -11,6 +11,7 @@ import {BehaviorSubject} from 'rxjs'; import { CdkPopoverEditColspan, + HoverContentState, PopoverEditClickOutBehavior, } from '@angular/cdk-experimental/popover-edit'; import {MatPopoverEditModule} from './index'; @@ -78,13 +79,24 @@ abstract class BaseTestComponent { row.dispatchEvent(new Event('mouseover', {bubbles: true})); row.dispatchEvent(new Event('mousemove', {bubbles: true})); - tick(31); + // Wait for the mouse hover debounce in edit-event-dispatcher. + tick(41); } getRows() { return getRows(this.table.nativeElement); } + hoverContentStateForRow(rowIndex = 0) { + const openButton = this.getOpenButton(rowIndex); + + if (!openButton) { + return HoverContentState.OFF; + } + return (openButton.parentNode as Element).classList.contains('mat-row-hover-content-visible') ? + HoverContentState.ON : HoverContentState.FOCUSABLE; + } + getEditCell(rowIndex = 0, cellIndex = 1) { const row = this.getRows()[rowIndex]; return getCells(row)[cellIndex]; @@ -266,7 +278,7 @@ describe('Material Popover Edit', () => { let fixture: ComponentFixture; let overlayContainer: OverlayContainer; - beforeEach(() => { + beforeEach(fakeAsync(() => { TestBed.configureTestingModule({ imports: [MatTableModule, MatPopoverEditModule, CommonModule, FormsModule], declarations: [componentClass], @@ -277,7 +289,8 @@ describe('Material Popover Edit', () => { fixture = TestBed.createComponent(componentClass); component = fixture.componentInstance; fixture.detectChanges(); - }); + tick(10); + })); afterEach(() => { // The overlay container's `ngOnDestroy` won't be called between test runs so we need @@ -286,41 +299,107 @@ describe('Material Popover Edit', () => { overlayContainer.ngOnDestroy(); }); + describe('row hover content', () => { + it('makes the first and last rows focusable but invisible', fakeAsync(() => { + const rows = component.getRows(); + + expect(component.hoverContentStateForRow(0)).toBe(HoverContentState.FOCUSABLE); + expect(component.hoverContentStateForRow(rows.length - 1)) + .toBe(HoverContentState.FOCUSABLE); + })); + }); + describe('triggering edit', () => { it('shows and hides on-hover content only after a delay', fakeAsync(() => { - const [row0, row1] = component.getRows(); - row0.dispatchEvent(new Event('mouseover', {bubbles: true})); - row0.dispatchEvent(new Event('mousemove', {bubbles: true})); + const [row0, row1] = component.getRows(); + row0.dispatchEvent(new Event('mouseover', {bubbles: true})); + row0.dispatchEvent(new Event('mousemove', {bubbles: true})); - expect(component.getOpenButton(0)).toBe(null); + expect(component.hoverContentStateForRow(0)).toBe(HoverContentState.FOCUSABLE); - tick(20); - row0.dispatchEvent(new Event('mousemove', {bubbles: true})); - tick(20); + tick(20); + row0.dispatchEvent(new Event('mousemove', {bubbles: true})); + tick(20); - expect(component.getOpenButton(0)).toBe(null); + expect(component.hoverContentStateForRow(0)).toBe(HoverContentState.FOCUSABLE); - tick(11); + tick(31); - expect(component.getOpenButton(0)).toEqual(jasmine.any(HTMLElement)); + expect(component.hoverContentStateForRow(0)).toBe(HoverContentState.ON); - row1.dispatchEvent(new Event('mouseover', {bubbles: true})); - row1.dispatchEvent(new Event('mousemove', {bubbles: true})); + row1.dispatchEvent(new Event('mouseover', {bubbles: true})); + row1.dispatchEvent(new Event('mousemove', {bubbles: true})); - expect(component.getOpenButton(0)).toEqual(jasmine.any(HTMLElement)); - expect(component.getOpenButton(1)).toBe(null); + expect(component.hoverContentStateForRow(0)).toBe(HoverContentState.ON); + expect(component.hoverContentStateForRow(1)).toBe(HoverContentState.OFF); - tick(31); + tick(41); - expect(component.getOpenButton(0)).toBe(null); - expect(component.getOpenButton(1)).toEqual(jasmine.any(HTMLElement)); - })); + expect(component.hoverContentStateForRow(0)).toBe(HoverContentState.FOCUSABLE); + expect(component.hoverContentStateForRow(1)).toBe(HoverContentState.ON); + })); + it('shows hover content for the focused row and makes the rows above and below focusable', + fakeAsync(() => { + expect(component.hoverContentStateForRow(1)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(2)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(3)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(4)).toBe(HoverContentState.FOCUSABLE); + + component.focusEditCell(2); + tick(1); + + expect(component.hoverContentStateForRow(2)).toBe(HoverContentState.ON); + expect(component.hoverContentStateForRow(1)).toBe(HoverContentState.FOCUSABLE); + expect(component.hoverContentStateForRow(3)).toBe(HoverContentState.FOCUSABLE); + + component.focusEditCell(4); + tick(1); + + expect(component.hoverContentStateForRow(2)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(4)).toBe(HoverContentState.ON); + expect(component.hoverContentStateForRow(3)).toBe(HoverContentState.FOCUSABLE); + + component.getEditCell(4).blur(); + tick(1); + + expect(component.hoverContentStateForRow(1)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(2)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(3)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(4)).toBe(HoverContentState.FOCUSABLE); + })); + + 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(() => { + expect(component.hoverContentStateForRow(1)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(2)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(3)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(4)).toBe(HoverContentState.FOCUSABLE); + + component.openLens(2); + tick(1); + + expect(component.hoverContentStateForRow(2)).toBe(HoverContentState.ON); + expect(component.hoverContentStateForRow(1)).toBe(HoverContentState.FOCUSABLE); + expect(component.hoverContentStateForRow(3)).toBe(HoverContentState.FOCUSABLE); + + component.focusEditCell(4); + tick(1); + + expect(component.hoverContentStateForRow(2)).toBe(HoverContentState.OFF); + expect(component.hoverContentStateForRow(4)).toBe(HoverContentState.ON); + expect(component.hoverContentStateForRow(3)).toBe(HoverContentState.FOCUSABLE); + })); + }); + + describe('triggering edit', () => { it('opens edit from on-hover button', fakeAsync(() => { component.triggerHoverState(); component.clickOpenButton(); expect(component.lensIsOpen()).toBe(true); + clearLeftoverTimers(); })); it('opens edit from Enter on focued cell', fakeAsync(() => { @@ -328,6 +407,7 @@ describe('Material Popover Edit', () => { component.openLens(); expect(component.lensIsOpen()).toBe(true); + clearLeftoverTimers(); })); }); @@ -416,6 +496,7 @@ describe('Material Popover Edit', () => { // Focus should have moved to the top of the lens. expect(document.activeElement).toBe(focusableElements[1]); expect(component.lensIsOpen()).toBe(true); + clearLeftoverTimers(); })); it('moves focus to the next cell when focus leaves end of lens with matPopoverEditTabOut', @@ -432,6 +513,7 @@ describe('Material Popover Edit', () => { // Focus should have moved to the next editable cell. expect(document.activeElement).toBe(component.getEditCell(1, 1)); expect(component.lensIsOpen()).toBe(false); + clearLeftoverTimers(); })); it(`moves focus to the previous cell when focus leaves end of lens with @@ -448,6 +530,7 @@ matPopoverEditTabOut`, fakeAsync(() => { // Focus should have moved to the next editable cell. expect(document.activeElement).toBe(component.getEditCell(0, 1)); expect(component.lensIsOpen()).toBe(false); + clearLeftoverTimers(); })); }); }); @@ -457,6 +540,7 @@ matPopoverEditTabOut`, fakeAsync(() => { component.openLens(); expect(component.getInput()!.value).toBe('Hydrogen'); + clearLeftoverTimers(); })); it('positions the lens at the top left corner and spans the full width of the cell', @@ -469,6 +553,7 @@ matPopoverEditTabOut`, fakeAsync(() => { expect(paneRect.width).toBe(cellRect.width); expect(paneRect.left).toBe(cellRect.left); expect(paneRect.top).toBe(cellRect.top); + clearLeftoverTimers(); })); it('adjusts the positioning of the lens based on colspan', fakeAsync(() => { @@ -500,6 +585,7 @@ matPopoverEditTabOut`, fakeAsync(() => { expect(paneRect.top).toBe(cellRects[0].top); expect(paneRect.left).toBe(cellRects[0].left); expect(paneRect.right).toBe(cellRects[2].right); + clearLeftoverTimers(); })); it('updates the form and submits, closing the lens', fakeAsync(() => { @@ -511,8 +597,9 @@ matPopoverEditTabOut`, fakeAsync(() => { component.clickSubmitButton(); fixture.detectChanges(); - expect(component.getEditCell().textContent!.trim()).toBe('Hydragon'); + expect(component.getEditCell().firstChild!.textContent!.trim()).toBe('Hydragon'); expect(component.lensIsOpen()).toBe(false); + clearLeftoverTimers(); })); it('does not close the lens on submit when form is invalid', fakeAsync(() => { @@ -524,6 +611,7 @@ matPopoverEditTabOut`, fakeAsync(() => { component.clickSubmitButton(); expect(component.lensIsOpen()).toBe(true); + clearLeftoverTimers(); })); it('closes lens on submit when form is invalid with ' + @@ -537,6 +625,7 @@ matPopoverEditTabOut`, fakeAsync(() => { component.clickSubmitButton(); expect(component.lensIsOpen()).toBe(false); + clearLeftoverTimers(); })); it('closes the lens on close', fakeAsync(() => { @@ -545,6 +634,7 @@ matPopoverEditTabOut`, fakeAsync(() => { component.clickCloseButton(); expect(component.lensIsOpen()).toBe(false); + clearLeftoverTimers(); })); it('closes and reopens a lens with modified value persisted', fakeAsync(() => { @@ -556,12 +646,13 @@ matPopoverEditTabOut`, fakeAsync(() => { component.clickCloseButton(); fixture.detectChanges(); - expect(component.getEditCell().textContent!.trim()).toBe('Hydrogen'); + expect(component.getEditCell().firstChild!.textContent!.trim()).toBe('Hydrogen'); expect(component.lensIsOpen()).toBe(false); component.openLens(); expect(component.getInput()!.value).toBe('Hydragon'); + clearLeftoverTimers(); })); it('resets the lens to original value', fakeAsync(() => { @@ -573,6 +664,7 @@ matPopoverEditTabOut`, fakeAsync(() => { component.clickRevertButton(); expect(component.getInput()!.value).toBe('Hydrogen'); + clearLeftoverTimers(); })); it('resets the lens to previously submitted value', fakeAsync(() => { @@ -592,6 +684,7 @@ matPopoverEditTabOut`, fakeAsync(() => { component.clickRevertButton(); expect(component.getInput()!.value).toBe('Hydragon'); + clearLeftoverTimers(); })); it('closes the lens on escape', fakeAsync(() => { @@ -601,6 +694,7 @@ matPopoverEditTabOut`, fakeAsync(() => { new KeyboardEvent('keyup', {bubbles: true, key: 'Escape'})); expect(component.lensIsOpen()).toBe(false); + clearLeftoverTimers(); })); it('does not close the lens on click within lens', fakeAsync(() => { @@ -609,6 +703,7 @@ matPopoverEditTabOut`, fakeAsync(() => { component.getInput()!.dispatchEvent(new Event('click', {bubbles: true})); expect(component.lensIsOpen()).toBe(true); + clearLeftoverTimers(); })); it('closes the lens on outside click', fakeAsync(() => { @@ -620,8 +715,8 @@ matPopoverEditTabOut`, fakeAsync(() => { fixture.detectChanges(); expect(component.lensIsOpen()).toBe(false); - expect(component.getEditCell().textContent!.trim()).toBe('Hydrogen'); - + expect(component.getEditCell().firstChild!.textContent!.trim()).toBe('Hydrogen'); + clearLeftoverTimers(); })); it('submits the lens on outside click with ' + @@ -635,7 +730,8 @@ matPopoverEditTabOut`, fakeAsync(() => { fixture.detectChanges(); expect(component.lensIsOpen()).toBe(false); - expect(component.getEditCell().textContent!.trim()).toBe('Hydragon'); + expect(component.getEditCell().firstChild!.textContent!.trim()).toBe('Hydragon'); + clearLeftoverTimers(); })); it('does nothing on outside click with ' + @@ -649,13 +745,15 @@ matPopoverEditTabOut`, fakeAsync(() => { fixture.detectChanges(); expect(component.lensIsOpen()).toBe(true); - expect(component.getEditCell().textContent!.trim()).toBe('Hydrogen'); + expect(component.getEditCell().firstChild!.textContent!.trim()).toBe('Hydrogen'); + clearLeftoverTimers(); })); it('sets focus on the first input in the lens', fakeAsync(() => { component.openLens(); expect(document.activeElement).toBe(component.getInput()); + clearLeftoverTimers(); })); it('returns focus to the edited cell after closing', fakeAsync(() => { @@ -664,6 +762,7 @@ matPopoverEditTabOut`, fakeAsync(() => { component.clickCloseButton(); expect(document.activeElement).toBe(component.getEditCell()); + clearLeftoverTimers(); })); it('does not focus to the edited cell after closing if another element ' + @@ -674,6 +773,7 @@ matPopoverEditTabOut`, fakeAsync(() => { component.getEditCell(1).dispatchEvent(new Event('click', {bubbles: true})); expect(document.activeElement).toBe(component.getEditCell(1)); + clearLeftoverTimers(); })); }); }); @@ -705,3 +805,10 @@ function getCells(row: Element): HTMLElement[] { return getElements(row, '.mat-cell, td'); } + +// Common actions like mouse events and focus/blur cause timers to be fired off. +// When not testing this behavior directly, use this function to clear any timers that were +// created in passing. +function clearLeftoverTimers() { + tick(100); +} diff --git a/src/material-experimental/popover-edit/table-directives.ts b/src/material-experimental/popover-edit/table-directives.ts index 2aacb91918c..ec7e7df3f29 100644 --- a/src/material-experimental/popover-edit/table-directives.ts +++ b/src/material-experimental/popover-edit/table-directives.ts @@ -7,6 +7,8 @@ */ import {Directive} from '@angular/core'; import { + _CELL_SELECTOR, + _closest, CdkPopoverEdit, CdkPopoverEditTabOut, CdkRowHoverContent, @@ -27,6 +29,11 @@ const POPOVER_EDIT_INPUTS = [ const EDIT_PANE_CLASS = 'mat-edit-pane'; +const MAT_ROW_HOVER_CLASS = 'mat-row-hover-content'; +const MAT_ROW_HOVER_RTL_CLASS = MAT_ROW_HOVER_CLASS + '-rtl'; +const MAT_ROW_HOVER_ANIMATE_CLASS = MAT_ROW_HOVER_CLASS + '-visible'; +const MAT_ROW_HOVER_CELL_CLASS = MAT_ROW_HOVER_CLASS + '-host-cell'; + /** * Attaches an ng-template to a cell and shows it when instructed to by the * EditEventDispatcher service. @@ -61,16 +68,7 @@ export class MatPopoverEditTabOut extends CdkPopoverEditTabOut { /** * A structural directive that shows its contents when the table row containing - * it is hovered. - * - * Note that the contents of this directive are invisible to screen readers. - * Typically this is used to show a button that launches the edit popup, which - * is ok because screen reader users can trigger edit by pressing Enter on a focused - * table cell. - * - * If this directive contains buttons for functionality other than opening edit then - * care should be taken to make sure that this functionality is also exposed in - * an accessible way. + * it is hovered or when an element in the row has focus. */ @Directive({ selector: '[matRowHoverContent]', @@ -78,24 +76,27 @@ export class MatPopoverEditTabOut extends CdkPopoverEditTabOut { export class MatRowHoverContent extends CdkRowHoverContent { protected initElement(element: HTMLElement) { super.initElement(element); - element.classList.add('mat-row-hover-content'); + element.classList.add(MAT_ROW_HOVER_CLASS); } - protected prepareElement(element: HTMLElement) { - super.prepareElement(element); + protected makeElementHiddenButFocusable(element: HTMLElement): void { + element.classList.remove(MAT_ROW_HOVER_ANIMATE_CLASS); + } + + protected makeElementVisible(element: HTMLElement): void { + _closest(this.elementRef.nativeElement!, _CELL_SELECTOR)! + .classList.add(MAT_ROW_HOVER_CELL_CLASS); - const RTL_CLASS = 'mat-row-hover-content-rtl'; if (this.services.directionality.value === 'rtl') { - element.classList.add(RTL_CLASS); + element.classList.add(MAT_ROW_HOVER_RTL_CLASS); } else { - element.classList.remove(RTL_CLASS); + element.classList.remove(MAT_ROW_HOVER_RTL_CLASS); } - const ANIMATE_CLASS = 'mat-row-hover-content-visible'; - element.classList.remove(ANIMATE_CLASS); + element.classList.remove(MAT_ROW_HOVER_ANIMATE_CLASS); this.services.ngZone.runOutsideAngular(() => { setTimeout(() => { - element.classList.add(ANIMATE_CLASS); + element.classList.add(MAT_ROW_HOVER_ANIMATE_CLASS); }); }); }