fix(popover-edit): hover content not showing up if content changes after init (#18937)

If the table's content changes after the first render, the popover edit wasn't showing up on hover, because some of the new subscribers weren't getting the previous value. These changes fix the issue by using `shareReplay` instead of `share`.

Fixes #18934.

Author: crisbeto

src/cdk-experimental/popover-edit/edit-event-dispatcher.ts

@@ -15,9 +15,9 @@ import {
   distinctUntilChanged,
   filter,
   map,
-  share,
   skip,
   startWith,
+  shareReplay,
 } from 'rxjs/operators';
 
 import {CELL_SELECTOR, ROW_SELECTOR} from './constants';
@@ -33,7 +33,7 @@ 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.
+ * FOCUSABLE - Rendered in the dom and styled for its contents to be focusable but invisible.
  * ON - Rendered and fully visible.
  */
 export const enum HoverContentState {
@@ -81,7 +81,7 @@ export class EditEventDispatcher {
   private readonly _startWithNull = startWith<Element|null>(null);
   private readonly _distinctShare = pipe(
     this._distinctUntilChanged as MonoTypeOperatorFunction<HoverContentState>,
-    share(),
+    shareReplay(1),
   );
   private readonly _startWithNullDistinct = pipe(
     this._startWithNull,
@@ -90,7 +90,7 @@ export class EditEventDispatcher {
 
   readonly editingAndEnabled = this.editing.pipe(
       filter(cell => cell == null || !this.disabledCells.has(cell)),
-      share(),
+      shareReplay(1),
   );
 
   /** An observable that emits the row containing focus or an active edit. */
@@ -105,7 +105,7 @@ export class EditEventDispatcher {
       this._distinctUntilChanged as MonoTypeOperatorFunction<Element|null>,
       auditTime(FOCUS_DELAY), // Use audit to skip over blur events to the next focused element.
       this._distinctUntilChanged as MonoTypeOperatorFunction<Element|null>,
-      share(),
+      shareReplay(1),
   );
 
   /** Tracks rows that contain hover content with a reference count. */
@@ -132,16 +132,16 @@ export class EditEventDispatcher {
       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
+      // Optimization: Enter the zone before shareReplay so that we trigger a single
       // ApplicationRef.tick for all row updates.
       this._enterZone(),
-      share(),
+      shareReplay(1),
   );
 
   private readonly _editingAndEnabledDistinct = this.editingAndEnabled.pipe(
       distinctUntilChanged(),
       this._enterZone(),
-      share(),
+      shareReplay(1),
   );
 
   // Optimization: Share row events observable with subsequent callers.

src/cdk-experimental/popover-edit/popover-edit.spec.ts

@@ -70,13 +70,18 @@ abstract class BaseTestComponent {
   @ViewChild('table') table: ElementRef;
 
   preservedValues = new FormValueContainer<PeriodicElement, {'name': string}>();
-
   nameEditDisabled = false;
   ignoreSubmitUnlessValid = true;
   clickOutBehavior: PopoverEditClickOutBehavior = 'close';
   colspan: CdkPopoverEditColspan = {};
   direction: Direction = 'ltr';
 
+  constructor() {
+    this.renderData();
+  }
+
+  abstract renderData(): void;
+
   onSubmit(element: PeriodicElement, form: NgForm) {
     if (!form.valid) { return; }
 
@@ -202,7 +207,11 @@ abstract class BaseTestComponent {
   `
 })
 class VanillaTableOutOfCell extends BaseTestComponent {
-  elements = createElementData();
+  elements: ChemicalElement[];
+
+  renderData() {
+    this.elements = createElementData();
+  }
 }
 
 @Component({
@@ -231,7 +240,11 @@ class VanillaTableOutOfCell extends BaseTestComponent {
   `
 })
 class VanillaTableInCell extends BaseTestComponent {
-  elements = createElementData();
+  elements: ChemicalElement[];
+
+  renderData() {
+    this.elements = createElementData();
+  }
 }
 
 class ElementDataSource extends DataSource<PeriodicElement> {
@@ -289,7 +302,11 @@ class ElementDataSource extends DataSource<PeriodicElement> {
 })
 class CdkFlexTableInCell extends BaseTestComponent {
   displayedColumns = ['before', 'name', 'weight'];
-  dataSource = new ElementDataSource();
+  dataSource: ElementDataSource;
+
+  renderData() {
+    this.dataSource = new ElementDataSource();
+  }
 }
 
 @Component({
@@ -335,7 +352,11 @@ class CdkFlexTableInCell extends BaseTestComponent {
 })
 class CdkTableInCell extends BaseTestComponent {
   displayedColumns = ['before', 'name', 'weight'];
-  dataSource = new ElementDataSource();
+  dataSource: ElementDataSource;
+
+  renderData() {
+    this.dataSource = new ElementDataSource();
+  }
 }
 
 const testCases: ReadonlyArray<[Type<BaseTestComponent>, string]> = [
@@ -369,7 +390,7 @@ describe('CDK Popover Edit', () => {
       afterEach(() => {
         // The overlay container's `ngOnDestroy` won't be called between test runs so we need
         // to call it ourselves, in order to avoid leaking containers between tests and potentially
-        // throwing `querySelector` calls.
+        // throwing off `querySelector` calls.
         overlayContainer.ngOnDestroy();
       });
 
@@ -411,6 +432,21 @@ describe('CDK Popover Edit', () => {
           expect(component.hoverContentStateForRow(1)).toBe(HoverContentState.ON);
         }));
 
+        it('shows the hover content if the data changes after initialization', fakeAsync(() => {
+          fixture.componentInstance.renderData();
+          fixture.detectChanges();
+
+          const row = component.getRows()[0];
+          row.dispatchEvent(new Event('mouseover', {bubbles: true}));
+          row.dispatchEvent(new Event('mousemove', {bubbles: true}));
+
+          tick(20);
+          row.dispatchEvent(new Event('mousemove', {bubbles: true}));
+          tick(50);
+
+          expect(component.hoverContentStateForRow(0)).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);
@@ -930,7 +966,12 @@ cdkPopoverEditTabOut`, fakeAsync(() => {
   }
 });
 
-function createElementData() {
+interface ChemicalElement {
+  name: string;
+  weight: number;
+}
+
+function createElementData(): ChemicalElement[] {
   return [
     {name: 'Hydrogen', weight: 1.007},
     {name: 'Helium', weight: 4.0026},