fix(diagram): keep trivial molecules visible while something is flagged

.github/workflows/tests.yml

@@ -48,16 +48,17 @@ jobs:
       # and passed every test. Without this step the only build was the deploy
       # job, so a broken template reached main and was found by a person.
       #
-      # reactome-cytoscape-style is built separately because build:libs does not
-      # cover it and styles.scss @use's it; content-dist is a gitignored asset
-      # input, so stage:content has to run or ng build cannot find it.
+      # content-dist is a gitignored asset input, so stage:content has to run or
+      # ng build cannot find it. The libraries no longer need a step of their
+      # own: `npm run build` builds them, because a build that does not produce
+      # what it links against is a build that can pass here and ship something
+      # else -- a fix to reactome-cytoscape-style reached CI and never reached
+      # the dev server, which builds the app without building the library.
       - name: Compile CMS content
         run: npm run stage:content
 
       - name: Build the app
-        run: |
-          npx ng build reactome-cytoscape-style
-          npm run build
+        run: npm run build
 
       # Lint errors must be zero -- eslint.config.js only marks a rule "error"
       # once the codebase is clean of it, so an error is always a new violation.
@@ -110,11 +111,7 @@ jobs:
       # pathway-browser build copies reactome-cytoscape-style's assets, so the
       # app cannot be served until they exist.
       - name: Build workspace libraries
-        run: |
-          npx ng build reactome-cytoscape-style
-          npx ng build ngx-reactome-style
-          npx ng build reactome-table
-          npx ng build reactome-gsa-form
+        run: npm run build:libs
 
       # site-search-index.json is generated, not committed, and the search
       # specs depend on it.

e2e/flagging-trivial.spec.ts

@@ -0,0 +1,134 @@
+import { test, expect, type Page } from '@playwright/test';
+
+/**
+ * Trivial molecules stay visible while something is flagged.
+ *
+ * Water and protons are drawn faintly and fade out as you zoom away, because a
+ * diagram carrying hundreds of them is unreadable. Flagging turns that off: a
+ * search has to be able to point at a molecule, and a molecule you cannot see
+ * is not an answer.
+ *
+ * Curators reported the opposite — "H2O and H+ disappear with zooming out" —
+ * and, separately, chemical structures drawn with no molecule underneath them,
+ * which is the same fault seen from the other side: the node was hidden while a
+ * different handler carried on drawing its structure.
+ *
+ * The cause was that the zoom handler writes an *inline* opacity, which in
+ * cytoscape beats any stylesheet rule, so it silently overrode the
+ * `.trivial.always-visible` rule flagging relies on. Detaching it from the zoom
+ * event was not enough: `triggerZoom()` calls it directly on every restyle and
+ * whenever an interactor opens.
+ */
+
+const BOOT_TIMEOUT = 60_000;
+// Glycolysis: 41 trivial elements, H+ and H2O among them.
+const PATHWAY = 'R-HSA-70171';
+const FLAGGED = 'PKM';
+
+/** The few cytoscape members these checks touch, so none of this needs `any`. */
+interface DiagramGraph {
+  zoom(level?: number): number;
+  emit(event: string): void;
+  elements(selector: string): {
+    length: number;
+    map<T>(fn: (element: { numericStyle(property: string): number }) => T): T[];
+  };
+  data(key: string): { update(graph: DiagramGraph): void };
+}
+
+/**
+ * Cytoscape registers itself on its own container so a second `cytoscape()`
+ * call cannot clobber it, and that registration is the only handle a test has.
+ * The alternative is reading opacities out of canvas pixels, which cannot say
+ * which element was faded.
+ */
+type CytoscapeHost = Element & { _cyreg?: { cy?: DiagramGraph } };
+
+async function trivialOpacities(page: Page): Promise<number[]> {
+  return page.evaluate(() => {
+    const host = document.querySelector('#cytoscape') as CytoscapeHost | null;
+    const cy = host?._cyreg?.cy;
+    if (!cy) throw new Error('no cytoscape instance on #cytoscape');
+    return [
+      ...new Set(
+        cy.elements('.trivial').map((element) => Number(element.numericStyle('opacity').toFixed(2)))
+      ),
+    ];
+  });
+}
+
+async function zoomTo(page: Page, zoom: number) {
+  await page.evaluate((level) => {
+    const cy = (document.querySelector('#cytoscape') as CytoscapeHost | null)?._cyreg?.cy;
+    if (!cy) throw new Error('no cytoscape instance on #cytoscape');
+    cy.zoom(level);
+    cy.emit('zoom');
+  }, zoom);
+  await page.waitForTimeout(400);
+}
+
+/** The flag arrives from its own request, after the diagram has drawn. */
+async function waitForFlag(page: Page) {
+  await page.waitForFunction(
+    () => {
+      const cy = (document.querySelector('#cytoscape') as CytoscapeHost | null)?._cyreg?.cy;
+      return cy ? cy.elements('.flag').length > 0 : false;
+    },
+    { timeout: BOOT_TIMEOUT }
+  );
+}
+
+async function openDiagram(page: Page, query = '') {
+  await page.goto(`/PathwayBrowser/${PATHWAY}${query}`);
+  await page.waitForSelector('#cytoscape canvas', { timeout: BOOT_TIMEOUT });
+  // The diagram keeps drawing after its first canvas, so wait for the elements
+  // themselves rather than for a delay.
+  await page.waitForFunction(
+    () => {
+      const cy = (document.querySelector('#cytoscape') as CytoscapeHost | null)?._cyreg?.cy;
+      return cy ? cy.elements('.trivial').length > 0 : false;
+    },
+    { timeout: BOOT_TIMEOUT }
+  );
+}
+
+test.describe('Trivial molecules and flagging', () => {
+  test.describe.configure({ timeout: 3 * 60 * 1000 });
+
+  test('fade with zoom when nothing is flagged', async ({ page }) => {
+    await openDiagram(page);
+
+    await zoomTo(page, 1.5);
+    expect(await trivialOpacities(page), 'visible close up').toEqual([1]);
+
+    await zoomTo(page, 0.14);
+    expect(Math.max(...(await trivialOpacities(page))), 'faint far out').toBeLessThan(1);
+  });
+
+  test('stay visible at every zoom once something is flagged', async ({ page }) => {
+    await openDiagram(page, `?FLG=${FLAGGED}`);
+    await waitForFlag(page);
+
+    expect(await trivialOpacities(page), 'as the diagram loads').toEqual([1]);
+
+    for (const zoom of [0.05, 1.4, 0.12]) {
+      await zoomTo(page, zoom);
+      expect(await trivialOpacities(page), `at zoom ${zoom}`).toEqual([1]);
+    }
+  });
+
+  test('survive a restyle, which runs the zoom handler directly', async ({ page }) => {
+    await openDiagram(page, `?FLG=${FLAGGED}`);
+    await waitForFlag(page);
+
+    // What a theme change or a loading analysis does.
+    await page.evaluate(() => {
+      const cy = (document.querySelector('#cytoscape') as CytoscapeHost | null)?._cyreg?.cy;
+      if (!cy) throw new Error('no cytoscape instance on #cytoscape');
+      cy.data('reactome').update(cy);
+    });
+    await page.waitForTimeout(500);
+
+    expect(await trivialOpacities(page), 'a restyle must not fade a flagged molecule').toEqual([1]);
+  });
+});

package.json

@@ -11,7 +11,7 @@
     "dev:serve": "wait-on dist/reactome-cytoscape-style/assets/index.scss && tinacms dev --rootPath projects/website-angular -c \"ng serve\"",
     "dev:serve:local": "wait-on dist/reactome-cytoscape-style/assets/index.scss && tinacms dev --rootPath projects/website-angular -c \"ng serve --configuration development\"",
     "build:reactome-cytoscape-style": "ng build reactome-cytoscape-style",
-    "build:libs": "ng build ngx-reactome-style && ng build reactome-table && ng build reactome-gsa-form",
+    "build:libs": "ng build reactome-cytoscape-style && ng build ngx-reactome-style && ng build reactome-table && ng build reactome-gsa-form",
     "start": "npm run generate:indices && npm run stage:content && npm run build:libs && run-p dev:reactome-cytoscape-style dev:serve",
     "start:local": "npm run generate:indices && run-p dev:reactome-cytoscape-style dev:serve:local",
     "start:simple": "ng serve",
@@ -20,8 +20,8 @@
     "start:simple:local": "ng serve --configuration development",
     "start:website": "cd projects/website-angular && npm run start",
     "start:pathway": "cd projects/pathway-browser && npm run start-with-deps",
-    "build": "npm run generate:indices && npm run stage:content && ng build --configuration production",
-    "build:curator": "npm run generate:indices && npm run stage:content && npm run build:reactome-cytoscape-style && npm run build:libs && ng build --configuration production,curator",
+    "build": "npm run build:libs && npm run generate:indices && npm run stage:content && ng build --configuration production",
+    "build:curator": "npm run build:libs && npm run generate:indices && npm run stage:content && ng build --configuration production,curator",
     "build:website": "npm run build:curator && cd dist/reactome/browser/ && tar czvf browser.tar * && scp browser.tgz curator:~/browser.tgz && rm browser.tar",
     "build:pathway": "cd projects/pathway-browser && npm run build",
     "watch": "ng build --watch --configuration development",

projects/reactome-cytoscape-style/src/lib/interactivity.ts

@@ -548,10 +548,25 @@ export class Interactivity {
       shadowLabels.style({
         'text-opacity': shadowLabelOpacity,
       });
-      trivial.style({
-        opacity: trivialOpacity,
-        'underlay-opacity': Math.min(shadowOpacity, trivialOpacity),
-      });
+      // Not the ones flagging has pinned visible.
+      //
+      // This writes an inline opacity, and in cytoscape an inline style beats
+      // any stylesheet rule -- so it silently overrode the `.trivial
+      // .always-visible` rule that flagging relies on. Detaching this handler
+      // from the zoom event was not enough to stop it: `triggerZoom()` calls it
+      // directly, and that runs on every restyle (a theme change, an analysis
+      // loading) and whenever an interactor is opened. Curators saw H2O and H+
+      // vanish as they zoomed out with something flagged, and saw chemical
+      // structures drawn with no molecule underneath -- the same fault, since
+      // the structures are drawn by a different handler that was still running.
+      //
+      // The class is the authority. Anything wearing it is left alone.
+      trivial
+        .filter((element) => !element.hasClass('always-visible'))
+        .style({
+          opacity: trivialOpacity,
+          'underlay-opacity': Math.min(shadowOpacity, trivialOpacity),
+        });
     };
     const updateDecorationPosition = (node: cytoscape.NodeSingular) => {
       if (!this.structureContainers.has(node)) return;