Fix workspace special casing

This fixes the special case for datagrids by forcing plugins to use perspective-select events rather than attempting to handle both click and selects. This also fixes some issues with global filtering on datagrids where selections overlapped with collapsing/expanding rows.

Signed-off-by: Timothy Bess <timbessmail@gmail.com>

Author: timbess

packages/viewer-d3fc/src/ts/tooltip/selectionEvent.ts

@@ -22,7 +22,7 @@ export const raiseEvent = (node, data, settings) => {
     const filter = settings.filter.concat(groupFilters).concat(splitFilters);
 
     node.dispatchEvent(
-        new CustomEvent("perspective-click", {
+        new CustomEvent("perspective-select", {
             bubbles: true,
             composed: true,
             detail: {

packages/viewer-datagrid/src/js/event_handlers/row_select_click.js

@@ -16,14 +16,17 @@ export async function selectionListener(
     regularTable,
     viewer,
     selected_rows_map,
+    /** @type {MouseEvent} */
     event,
 ) {
     const meta = regularTable.getMeta(event.target);
     if (!viewer.hasAttribute("selectable")) return;
     if (event.handled) return;
-    if (event.which !== 1) {
+    if (event.shiftKey) return;
+    if (event.button !== 0) {
         return;
     }
+    event.stopImmediatePropagation();
 
     if (!meta) {
         return;

packages/viewer-datagrid/src/js/plugin/activate.js

@@ -91,12 +91,6 @@ export async function activate(view) {
             click_listener.bind(this.model, this.regular_table),
         );
 
-        // tree collapse, expand, edit button headers
-        this.regular_table.addEventListener(
-            "mousedown",
-            mousedown_listener.bind(this.model, this.regular_table, viewer),
-        );
-
         // (Legacy) Row selection
         const selected_rows_map = new WeakMap();
         this.regular_table.addStyleListener(
@@ -138,6 +132,12 @@ export async function activate(view) {
             ),
         );
 
+        // tree collapse, expand, edit button headers
+        this.regular_table.addEventListener(
+            "mousedown",
+            mousedown_listener.bind(this.model, this.regular_table, viewer),
+        );
+
         // Editing
         const selected_position_map = new WeakMap();
         this.regular_table.addStyleListener(

packages/workspace/src/ts/workspace/workspace.ts

@@ -358,11 +358,6 @@ export class PerspectiveWorkspace extends SplitPanel {
                 this.onPerspectiveSelect.bind(this),
             );
 
-            widget.viewer.addEventListener(
-                "perspective-click",
-                this.onPerspectiveSelect.bind(this),
-            );
-
             // TODO remove event listener
             this.masterPanel.addWidget(widget);
         }
@@ -556,14 +551,7 @@ export class PerspectiveWorkspace extends SplitPanel {
         const config = await (
             event.target as HTMLPerspectiveViewerElement
         ).save();
-        // perspective-select is already handled for hypergrid
 
-        if (
-            event.type === "perspective-click" &&
-            (config.plugin === "Datagrid" || config.plugin === null)
-        ) {
-            return;
-        }
         const candidates = new Set([
             ...(config["group_by"] || []),
             ...(config["split_by"] || []),
@@ -595,11 +583,6 @@ export class PerspectiveWorkspace extends SplitPanel {
         this.masterPanel.addWidget(widget);
         widget.isHidden && widget.show();
         widget.viewer.restyleElement();
-        widget.viewer.addEventListener(
-            "perspective-click",
-            this.onPerspectiveSelect.bind(this),
-        );
-
         widget.viewer.addEventListener(
             "perspective-select",
             this.onPerspectiveSelect.bind(this),
@@ -618,11 +601,6 @@ export class PerspectiveWorkspace extends SplitPanel {
         }
 
         widget.viewer.restyleElement();
-        widget.viewer.removeEventListener(
-            "perspective-click",
-            this.onPerspectiveSelect.bind(this),
-        );
-
         widget.viewer.removeEventListener(
             "perspective-select",
             this.onPerspectiveSelect.bind(this),

packages/workspace/test/js/global_filter.spec.js

@@ -98,35 +98,142 @@ function tests(context, compare) {
             },
         };
 
-        const cfg = await page.evaluate(async (config) => {
+        async function awaitConfigChange() {
+            return await page.evaluate(async () => {
+                let resolve;
+                const timer = new Promise((x) => {
+                    resolve = x;
+                });
+
+                workspace.addEventListener("workspace-layout-update", resolve);
+                await timer;
+                workspace.removeEventListener(
+                    "workspace-layout-update",
+                    resolve,
+                );
+
+                return await workspace.save();
+            });
+        }
+
+        await page.evaluate(async (config) => {
             const workspace = document.getElementById("workspace");
             await workspace.restore(config);
             await workspace.flush();
-            const event = new Event("mousedown", { bubbles: true });
-            event.which = 1;
+        }, config);
 
-            document
-                .querySelector(
-                    ".workspace-master-widget perspective-viewer-datagrid",
-                )
-                .shadowRoot.querySelector(
-                    "tbody tr:nth-child(6) th:last-of-type",
-                )
-                .dispatchEvent(event);
+        let cfgPromise = awaitConfigChange();
+        await page
+            .locator(".workspace-master-widget perspective-viewer-datagrid")
+            .locator("tbody tr:nth-child(6) th:last-of-type")
+            .click();
+        let cfg = await cfgPromise;
 
-            let resolve;
-            const timer = new Promise((x) => {
-                resolve = x;
-            });
+        expect(cfg.viewers.Two.filter).toEqual([["State", "==", "Colorado"]]);
 
-            workspace.addEventListener("workspace-layout-update", resolve);
-            await timer;
+        cfgPromise = awaitConfigChange();
+        await page
+            .locator(".workspace-master-widget perspective-viewer-datagrid")
+            .locator("tbody tr:nth-child(6) th:last-of-type")
+            .click();
+        cfg = await cfgPromise;
 
-            return await workspace.save();
+        expect(cfg.viewers.Two.filter).toEqual([]);
+
+        return compare(page, `${context}-datagrid-filters-work.txt`);
+    });
+
+    test("Child classes of datagrid behave the same way", async ({ page }) => {
+        const config = {
+            viewers: {
+                One: {
+                    table: "superstore",
+                    name: "Test",
+                    group_by: ["State"],
+                    columns: ["Sales"],
+                    plugin: "My Datagrid",
+                },
+                Two: { table: "superstore", name: "One" },
+            },
+            master: {
+                widgets: ["One"],
+            },
+            detail: {
+                main: {
+                    currentIndex: 0,
+                    type: "tab-area",
+                    widgets: ["Two"],
+                },
+            },
+        };
+
+        await page.evaluate(async () => {
+            class MyGrid extends customElements.get(
+                "perspective-viewer-datagrid",
+            ) {
+                get name() {
+                    return "My Datagrid";
+                }
+            }
+            customElements.define("my-grid", MyGrid);
+            customElements.get("perspective-viewer").registerPlugin("my-grid");
+        });
+
+        async function awaitConfigChange() {
+            return await page.evaluate(async () => {
+                let resolve;
+                const timer = new Promise((x) => {
+                    resolve = x;
+                });
+
+                workspace.addEventListener("workspace-layout-update", resolve);
+                await timer;
+                workspace.removeEventListener(
+                    "workspace-layout-update",
+                    resolve,
+                );
+
+                return await workspace.save();
+            });
+        }
+
+        await page.evaluate(async (config) => {
+            const workspace = document.getElementById("workspace");
+            await workspace.restore(config);
+            await workspace.flush();
         }, config);
 
+        let cfgPromise = awaitConfigChange();
+        await page
+            .locator(".workspace-master-widget my-grid")
+            .locator("tbody tr:nth-child(6) th:last-of-type")
+            .click();
+        let cfg = await cfgPromise;
+
         expect(cfg.viewers.Two.filter).toEqual([["State", "==", "Colorado"]]);
-        return compare(page, `${context}-datagrid-filters-work.txt`);
+
+        cfgPromise = awaitConfigChange();
+        await page
+            .locator(".workspace-master-widget my-grid")
+            .locator("tbody tr:nth-child(6) th:last-of-type")
+            .click({
+                delay: 10,
+            });
+        cfg = await cfgPromise;
+
+        // console.log("OLD", cfg.viewers.Two.filter);
+        // await page.evaluate(async () => {
+        //     await new Promise((r) => setTimeout(r, 5000));
+        // });
+        // cfg = await page.evaluate(async () => {
+        //     const cfg = await workspace.save();
+        //     console.log("NEW", JSON.stringify(cfg.viewers.Two.filter));
+        //     return cfg;
+        // });
+
+        expect(cfg.viewers.Two.filter).toEqual([]);
+
+        return compare(page, `${context}-my-datagrid-filters-work.txt`);
     });
 }