Merge pull request #1950 from finos/fix-style-menus-regression

Fix column style menus for string & date(time)
finos · Aug 25, 2022 · eaf03bb · eaf03bb
2 parents ca2c560 + 084e440
commit eaf03bb
Show file tree

Hide file tree

Showing 4 changed files with 200 additions and 88 deletions.
diff --git a/packages/perspective-viewer-datagrid/test/js/column_style.spec.js b/packages/perspective-viewer-datagrid/test/js/column_style.spec.js
@@ -0,0 +1,195 @@
+/******************************************************************************
+ *
+ * Copyright (c) 2017, the Perspective Authors.
+ *
+ * This file is part of the Perspective library, distributed under the terms of
+ * the Apache License 2.0.  The full license can be found in the LICENSE file.
+ *
+ */
+
+const utils = require("@finos/perspective-test");
+const path = require("path");
+
+const simple_tests = require("@finos/perspective-viewer/test/js/simple_tests.js");
+
+async function get_contents(
+    page,
+    selector = "perspective-viewer perspective-viewer-datagrid regular-table",
+    shadow = false
+) {
+    return await page.evaluate(
+        async (selector, shadow) => {
+            const viewer = document.querySelector(selector);
+            return (shadow ? viewer.shadowRoot : viewer).innerHTML || "MISSING";
+        },
+        selector,
+        shadow
+    );
+}
+
+utils.with_server({}, () => {
+    describe.page(
+        "superstore.html",
+        () => {
+            test.capture(
+                "perspective-config-update event is fired when column style is changed",
+                async (page) => {
+                    // Await the viewer element to exist on the page
+                    const viewer = await page.waitForSelector(
+                        "perspective-viewer"
+                    );
+                    const {x, y} = await page.evaluate(async (viewer) => {
+                        // Await the table load
+                        await viewer.getTable();
+
+                        // Open the config panel
+                        await viewer.toggleConfig();
+
+                        // Register a listener for `perspective-config-update` event
+                        window.__events__ = [];
+                        viewer.addEventListener(
+                            "perspective-config-update",
+                            (evt) => {
+                                window.__events__.push(evt);
+                            }
+                        );
+
+                        // Find the column config menu button
+                        const header_button = viewer.querySelector(
+                            "regular-table thead tr:last-child th"
+                        );
+
+                        // Get the button coords (slightly lower than center
+                        // because of the location of the menu button within
+                        // this element)
+                        const rect = header_button.getBoundingClientRect();
+                        return {
+                            x: Math.floor(rect.left + rect.width / 2),
+                            y: Math.floor(rect.top + (3 * rect.height) / 4),
+                        };
+                    }, viewer);
+
+                    // Click the menu button
+                    await page.mouse.click(x, y);
+
+                    // Await the style menu existing on the page
+                    const style_menu = await page.waitForSelector(
+                        "perspective-number-column-style"
+                    );
+
+                    const {x: xx, y: yy} = await page.evaluate(
+                        async (style_menu) => {
+                            // Find the 'bar' button
+                            const bar_button =
+                                style_menu.shadowRoot.querySelector(
+                                    '#radio-list-1[name="foreground-list"]'
+                                );
+
+                            // Get its coords
+                            const rect = bar_button.getBoundingClientRect();
+                            return {
+                                x: Math.floor(rect.left + rect.width / 2),
+                                y: Math.floor(rect.top + rect.height / 2),
+                            };
+                        },
+                        style_menu
+                    );
+
+                    // Click the button
+                    await page.mouse.click(xx, yy);
+
+                    const count = await page.evaluate(async (viewer) => {
+                        // Await the plugin rendering
+                        await viewer.flush();
+
+                        // Count the events;
+                        return window.__events__.length;
+                    }, viewer);
+
+                    // Expect 1 event
+                    expect(count).toEqual(1);
+
+                    // Return the `<table>` contents
+                    return get_contents(page);
+                }
+            );
+
+            describe("Column style menu opens for", () => {
+                async function test_column(page, selector, selector2) {
+                    const viewer = await page.waitForSelector(
+                        "perspective-viewer"
+                    );
+                    const {x, y} = await page.evaluate(
+                        async (viewer, selector) => {
+                            await viewer.getTable();
+                            await viewer.toggleConfig();
+                            window.__events__ = [];
+                            viewer.addEventListener(
+                                "perspective-config-update",
+                                (evt) => {
+                                    window.__events__.push(evt);
+                                }
+                            );
+
+                            const header_button = viewer.querySelector(
+                                "regular-table thead tr:last-child th" +
+                                    selector
+                            );
+
+                            const rect = header_button.getBoundingClientRect();
+                            return {
+                                x: Math.floor(rect.left + rect.width / 2),
+                                y: Math.floor(rect.top + (3 * rect.height) / 4),
+                            };
+                        },
+                        viewer,
+                        selector
+                    );
+
+                    await page.mouse.click(x, y);
+                    const style_menu = await page.waitForSelector(
+                        `perspective-${selector2}-column-style`
+                    );
+
+                    await new Promise((x) => setTimeout(x, 3000));
+
+                    return get_contents(
+                        page,
+                        ` perspective-${selector2}-column-style`,
+                        true
+                    );
+                }
+
+                test.capture("numeric columns", async (page) => {
+                    return await test_column(page, "", "number");
+                });
+
+                test.capture("string columns", async (page) => {
+                    return await test_column(page, ":nth-child(2)", "string");
+                });
+
+                // TODO Intl.supportedValuesOf doesn't exist in Chromium
+                test.skip("date columns", async (page) => {
+                    await test_column(page, ":nth-child(3)", "datetime");
+                });
+            });
+        },
+        {root: path.join(__dirname, "..", "..")}
+    );
+});
+
+// const click_details = async (page, x = 310, y = 300) => {
+//     const viewer = await page.$("perspective-viewer");
+
+//     const click_event = page.evaluate(
+//         element =>
+//             new Promise(resolve => {
+//                 element.addEventListener("perspective-click", e => {
+//                     resolve(e.detail);
+//                 });
+//             }),
+//         viewer
+//     );
+//     await page.mouse.click(x, y);
+//     return await click_event;
+// };
diff --git a/packages/perspective-viewer-datagrid/test/js/superstore.spec.js b/packages/perspective-viewer-datagrid/test/js/superstore.spec.js
@@ -27,89 +27,6 @@ utils.with_server({}, () => {
         () => {
             simple_tests.default(get_contents);
 
-            test.capture(
-                "perspective-config-update event is fired when column style is changed",
-                async (page) => {
-                    // Await the viewer element to exist on the page
-                    const viewer = await page.waitForSelector(
-                        "perspective-viewer"
-                    );
-                    const {x, y} = await page.evaluate(async (viewer) => {
-                        // Await the table load
-                        await viewer.getTable();
-
-                        // Open the config panel
-                        await viewer.toggleConfig();
-
-                        // Register a listener for `perspective-config-update` event
-                        window.__events__ = [];
-                        viewer.addEventListener(
-                            "perspective-config-update",
-                            (evt) => {
-                                window.__events__.push(evt);
-                            }
-                        );
-
-                        // Find the column config menu button
-                        const header_button = viewer.querySelector(
-                            "regular-table thead tr:last-child th"
-                        );
-
-                        // Get the button coords (slightly lower than center
-                        // because of the location of the menu button within
-                        // this element)
-                        const rect = header_button.getBoundingClientRect();
-                        return {
-                            x: Math.floor(rect.left + rect.width / 2),
-                            y: Math.floor(rect.top + (3 * rect.height) / 4),
-                        };
-                    }, viewer);
-
-                    // Click the menu button
-                    await page.mouse.click(x, y);
-
-                    // Await the style menu existing on the page
-                    const style_menu = await page.waitForSelector(
-                        "perspective-number-column-style"
-                    );
-
-                    const {x: xx, y: yy} = await page.evaluate(
-                        async (style_menu) => {
-                            // Find the 'bar' button
-                            const bar_button =
-                                style_menu.shadowRoot.querySelector(
-                                    '#radio-list-1[name="foreground-list"]'
-                                );
-
-                            // Get its coords
-                            const rect = bar_button.getBoundingClientRect();
-                            return {
-                                x: Math.floor(rect.left + rect.width / 2),
-                                y: Math.floor(rect.top + rect.height / 2),
-                            };
-                        },
-                        style_menu
-                    );
-
-                    // Click the button
-                    await page.mouse.click(xx, yy);
-
-                    const count = await page.evaluate(async (viewer) => {
-                        // Await the plugin rendering
-                        await viewer.flush();
-
-                        // Count the events;
-                        return window.__events__.length;
-                    }, viewer);
-
-                    // Expect 1 event
-                    expect(count).toEqual(1);
-
-                    // Return the `<table>` contents
-                    return get_contents(page);
-                }
-            );
-
             // test.capture("resets viewable area when the logical size expands.", async page => {
             //     const viewer = await page.$("perspective-viewer");
             //     await page.evaluate(async () => await document.querySelector("perspective-viewer").toggleConfig());

diff --git a/packages/perspective-viewer-datagrid/test/results/results.json b/packages/perspective-viewer-datagrid/test/results/results.json
@@ -14,7 +14,7 @@
     "superstore_displays_visible_columns_": "09dd14f0e60da0e8749b7dd514e929f0",
     "superstore_resets_viewable_area_when_the_logical_size_expands_": "e9dd1d275f46f6a0857e0168835d0b38",
     "superstore_resets_viewable_area_when_the_physical_size_expands_": "e469597235cc032619bc095a76f9ad6f",
-    "__GIT_COMMIT__": "c744efa438393bf46bf0d0801d9807fd86b64f51",
+    "__GIT_COMMIT__": "9c485e5c84b2e9b029fc13465390e14103493d66",
     "superstore_shows_a_grid_without_any_settings_applied": "192edc6b035af6432c5bd82d295987f1",
     "superstore_pivot_by_a_row": "709a4a6b40153d9bf013f90398ee568d",
     "superstore_pivot_by_two_rows": "23db46ccf9d8327ae992fe3b5f468b3b",
@@ -27,5 +27,7 @@
     "superstore_filters_filters_by_a_numeric_column": "6c6ba00b6edf9ee11f82848b7786fd91",
     "superstore_filters_filters_by_an_alpha_column": "4b9a40c2ffad050ff4cf059e22cb2b68",
     "superstore_filters_filters_with__in__comparator": "5882e2d2694519ba9880d0b2d5b19c44",
-    "superstore_perspective-config-update_event_is_fired_when_column_style_is_changed": "9c6f8055955b560ac1418554c01046fc"
+    "superstore_perspective-config-update_event_is_fired_when_column_style_is_changed": "3d03b2787f9b903050942052168f755a",
+    "superstore_Column_style_menu_opens_for_numeric_columns": "8d4449c817e262b1a6e435912200f58c",
+    "superstore_Column_style_menu_opens_for_string_columns": "b7eff686a96a57de363b3dd431032f40"
 }
diff --git a/rust/perspective-viewer/src/rust/components/style/style_cache.rs b/rust/perspective-viewer/src/rust/components/style/style_cache.rs
@@ -56,9 +56,7 @@ impl StyleCache {
             let first = map.values().next().cloned();
             map.insert(name, style.clone());
             if let Some(x) = first {
-                if x.is_connected() {
-                    x.get_root_node().insert_before(&style, Some(&x)).unwrap();
-                }
+                x.get_root_node().insert_before(&style, Some(&x)).unwrap();
             }
         }
     }