fix(hierarchy): hold the tree's place when a deep sub-event is clicked

e2e/hierarchy-scroll.spec.ts

@@ -0,0 +1,131 @@
+import { test, expect, type Page } from '@playwright/test';
+
+/**
+ * The event hierarchy holds its place when you click something in it.
+ *
+ * Curators, re-testing #137: "no jumping in the search and the analysis table
+ * and none in the upper section of event hierarchy, but pathways near the bottom
+ * of the hierarchy (e.g. subevents) still jump."
+ *
+ * They were right, and the cause was not the revealing. Rebuilding the tree
+ * starts by emptying it -- a workaround for an Angular Material bug where nested
+ * children otherwise do not render -- and emptying it destroys every row, so the
+ * container collapses and the browser resets the scroll to the top. Measured on
+ * a deep sub-event: 274px, then 0, then 6. Expansion state was already carried
+ * across that rebuild; the scroll position was not.
+ *
+ * Both halves matter, so both are checked: clicking a row you can already see
+ * must not move the tree, and selecting one you cannot see must still bring it
+ * into view.
+ */
+
+const BOOT_TIMEOUT = 90_000;
+// Intrinsic Pathway for Apoptosis, with its ancestors expanded.
+const DEEP = '/PathwayBrowser/R-HSA-109606?path=R-HSA-5357801,R-HSA-109581';
+const TREE = 'cr-event-hierarchy';
+const SCROLLER = '#events-container';
+
+async function openDeepHierarchy(page: Page) {
+  await page.goto(DEEP);
+  await page.waitForSelector('#cytoscape canvas', { timeout: BOOT_TIMEOUT });
+  await page.waitForSelector(`${TREE} [role="treeitem"]`, { timeout: BOOT_TIMEOUT });
+  await page.waitForTimeout(3000);
+
+  // Open whatever is still closed, so there is something below the fold.
+  for (let round = 0; round < 3; round++) {
+    const toggles = page.locator(`${TREE} [role="treeitem"] button`);
+    const count = await toggles.count();
+    let opened = 0;
+    for (let index = 0; index < count && opened < 6; index++) {
+      const toggle = toggles.nth(index);
+      if ((await toggle.getAttribute('aria-expanded').catch(() => null)) === 'false') {
+        await toggle.click({ timeout: 5000 }).catch(() => undefined);
+        opened++;
+        await page.waitForTimeout(250);
+      }
+    }
+    if (!opened) break;
+  }
+  await page.waitForTimeout(1000);
+}
+
+/** Scroll to the bottom and mark a row that is comfortably in view there. */
+async function markVisibleRowNearBottom(page: Page) {
+  return page.evaluate(async (selector) => {
+    const scroller = document.querySelector(selector) as HTMLElement | null;
+    if (!scroller) return null;
+    scroller.scrollTop = scroller.scrollHeight;
+    await new Promise((resolve) => setTimeout(resolve, 400));
+
+    const box = scroller.getBoundingClientRect();
+    const rows = [...document.querySelectorAll('cr-event-hierarchy [role="treeitem"]')].filter(
+      (row) => {
+        const rect = row.getBoundingClientRect();
+        return rect.top > box.top + 20 && rect.bottom < box.bottom - 20;
+      }
+    );
+    const pick = rows[Math.floor(rows.length / 2)];
+    if (!pick) return null;
+    pick.setAttribute('data-pick', '1');
+    return { scrollTop: Math.round(scroller.scrollTop), label: (pick.textContent ?? '').trim() };
+  }, SCROLLER);
+}
+
+const scrollTop = (page: Page) =>
+  page.evaluate(
+    (selector) => Math.round((document.querySelector(selector) as HTMLElement).scrollTop),
+    SCROLLER
+  );
+
+test.describe('Event hierarchy scrolling', () => {
+  test.describe.configure({ timeout: 4 * 60 * 1000 });
+
+  test('does not move when you click a row you can already see', async ({ page }) => {
+    await openDeepHierarchy(page);
+
+    const marked = await markVisibleRowNearBottom(page);
+    test.skip(!marked || marked.scrollTop === 0, 'the hierarchy here does not scroll');
+
+    const before = await scrollTop(page);
+    await page.locator(`${TREE} [data-pick="1"]`).click();
+    // Long enough for the rebuild, the expansion restore and any revealing.
+    await page.waitForTimeout(2500);
+    const after = await scrollTop(page);
+
+    // This was 274 -> 0 -> 6.
+    expect(Math.abs(after - before), `before ${before}, after ${after}`).toBeLessThan(24);
+  });
+
+  test('still brings a row you cannot see into view', async ({ page }) => {
+    await openDeepHierarchy(page);
+
+    const target = await page.evaluate(async (selector) => {
+      const scroller = document.querySelector(selector) as HTMLElement;
+      scroller.scrollTop = scroller.scrollHeight;
+      await new Promise((resolve) => setTimeout(resolve, 400));
+      const box = scroller.getBoundingClientRect();
+      const above = [...document.querySelectorAll('cr-event-hierarchy [role="treeitem"]')].filter(
+        (row) => row.getBoundingClientRect().bottom < box.top
+      );
+      const pick = above[Math.floor(above.length / 2)];
+      if (!pick) return null;
+      pick.setAttribute('data-target', '1');
+      return { scrollTop: Math.round(scroller.scrollTop), above: above.length };
+    }, SCROLLER);
+
+    test.skip(!target || target.above === 0, 'nothing is scrolled out of view here');
+
+    const before = await scrollTop(page);
+    // It is off-screen, so dispatch the click rather than asking Playwright to
+    // scroll it into view first -- that would defeat the test.
+    await page.evaluate(() =>
+      document
+        .querySelector('[data-target="1"]')
+        ?.dispatchEvent(new MouseEvent('click', { bubbles: true }))
+    );
+    await page.waitForTimeout(2500);
+    const after = await scrollTop(page);
+
+    expect(after, `it should have scrolled up from ${before}`).toBeLessThan(before - 20);
+  });
+});

projects/pathway-browser/src/app/event-hierarchy/event-hierarchy.component.html

@@ -37,7 +37,7 @@
     </nav>
   </div>
 
-  <div id="events-container">
+  <div id="events-container" #eventsContainer>
     <mat-tree
       #tree
       [dataSource]="treeDataSource"

projects/pathway-browser/src/app/event-hierarchy/event-hierarchy.component.ts

@@ -9,6 +9,7 @@ import {
   model,
   OnDestroy,
   ViewChild,
+  viewChild,
 } from '@angular/core';
 import { Event } from '../model/graph/event/event.model';
 import { EventService, SelectableObject } from '../services/event.service';
@@ -121,6 +122,9 @@ export class EventHierarchyComponent implements AfterViewInit, OnDestroy {
     return [];
   };
 
+  /** The element that scrolls, so its position can survive a tree rebuild. */
+  private readonly eventsContainer = viewChild<ElementRef<HTMLElement>>('eventsContainer');
+
   treeDataSource = new MatTreeNestedDataSource<Event>();
 
   breadcrumbs: Event[] = [];
@@ -218,12 +222,29 @@ export class EventHierarchyComponent implements AfterViewInit, OnDestroy {
     this.eventService.treeData$.pipe(untilDestroyed(this)).subscribe((events) => {
       // Save expanded node stIds before resetting the data source
       const expandedIds = this.collectExpandedIds(this.treeDataSource.data);
+      // The workaround below empties the tree, which destroys every row: the
+      // container collapses to nothing and the browser resets its scroll to the
+      // top. Clicking a sub-event near the bottom of the hierarchy therefore
+      // jumped to the top and scrolled back -- measured at 274px, then 0, then
+      // 6. Expansion state is already carried across this rebuild; the scroll
+      // position has to be carried the same way.
+      const scroller = this.eventsContainer()?.nativeElement;
+      const scrollTop = scroller?.scrollTop ?? 0;
       // Mat tree has a bug causing children to not be rendered in the UI without first setting the data to null
       // This is a workaround to add child data to tree and update the view. see details: https://github.com/angular/components/issues/11381
       this.treeDataSource.data = []; //todo: check performance issue
       this.treeDataSource.data = events as Event[];
       // Restore expansion state
       this.restoreExpandedIds(events as Event[], expandedIds);
+      if (scroller && scrollTop > 0) {
+        scroller.scrollTop = scrollTop;
+        // Again once the rows have been laid out: the height is not final until
+        // the restored branches have rendered, and the browser clamps a
+        // scrollTop set against a container that is still short.
+        requestAnimationFrame(() => {
+          if (scroller.scrollTop !== scrollTop) scroller.scrollTop = scrollTop;
+        });
+      }
       this.adjustWidths();
     });