Fix #1213 - improve render warning behavior, add tests

finos · Nov 7, 2020 · 3f336f4 · 3f336f4
1 parent 6ebb74a
commit 3f336f4
Show file tree

Hide file tree

Showing 24 changed files with 179 additions and 25 deletions.
diff --git a/packages/perspective-viewer-d3fc/package.json b/packages/perspective-viewer-d3fc/package.json
@@ -34,7 +34,7 @@
         "build": "npm-run-all --silent build:babel build:webpack:cjs build:webpack:umd",
         "test:build": "cpx \"test/html/*\" dist/umd",
         "watch": "webpack --color --watch --config src/config/d3fc.watch.config.js",
-        "test:run": "jest --rootDir=. --config=../perspective-test/jest.config.js --silent --color --noStackTrace 2>&1",
+        "test:run": "jest --rootDir=. --config=../perspective-test/jest.config.js --color --noStackTrace 2>&1",
         "test": "npm-run-all test:build test:run",
         "clean": "rimraf dist",
         "clean:screenshots": "rimraf \"screenshots/**/*.@(failed|diff).png\""

diff --git a/packages/perspective-viewer-d3fc/src/js/charts/area.js b/packages/perspective-viewer-d3fc/src/js/charts/area.js
@@ -88,7 +88,8 @@ areaChart.plugin = {
     type: "d3_y_area",
     name: "Y Area Chart",
     max_cells: 4000,
-    max_columns: 50
+    max_columns: 50,
+    render_warning: true
 };
 
 export default areaChart;
diff --git a/packages/perspective-viewer-d3fc/src/js/charts/bar.js b/packages/perspective-viewer-d3fc/src/js/charts/bar.js
@@ -68,7 +68,8 @@ barChart.plugin = {
     type: "d3_x_bar",
     name: "X Bar Chart",
     max_cells: 1000,
-    max_columns: 50
+    max_columns: 50,
+    render_warning: true
 };
 
 export default barChart;
diff --git a/packages/perspective-viewer-d3fc/src/js/charts/candlestick.js b/packages/perspective-viewer-d3fc/src/js/charts/candlestick.js
@@ -15,6 +15,7 @@ candlestick.plugin = {
     name: "Candlestick Chart",
     max_cells: 4000,
     max_columns: 50,
+    render_warning: true,
     initial: {
         type: "number",
         count: 4,

diff --git a/packages/perspective-viewer-d3fc/src/js/charts/column.js b/packages/perspective-viewer-d3fc/src/js/charts/column.js
@@ -88,7 +88,8 @@ columnChart.plugin = {
     type: "d3_y_bar",
     name: "Y Bar Chart",
     max_cells: 1000,
-    max_columns: 50
+    max_columns: 50,
+    render_warning: true
 };
 
 export default columnChart;
diff --git a/packages/perspective-viewer-d3fc/src/js/charts/heatmap.js b/packages/perspective-viewer-d3fc/src/js/charts/heatmap.js
@@ -62,7 +62,8 @@ heatmapChart.plugin = {
     type: "d3_heatmap",
     name: "Heatmap",
     max_cells: 1000,
-    max_columns: 50
+    max_columns: 50,
+    render_warning: true
 };
 
 export default heatmapChart;
diff --git a/packages/perspective-viewer-d3fc/src/js/charts/line.js b/packages/perspective-viewer-d3fc/src/js/charts/line.js
@@ -97,7 +97,8 @@ lineChart.plugin = {
     type: "d3_y_line",
     name: "Y Line Chart",
     max_cells: 4000,
-    max_columns: 50
+    max_columns: 50,
+    render_warning: true
 };
 
 export default lineChart;
diff --git a/packages/perspective-viewer-d3fc/src/js/charts/ohlc.js b/packages/perspective-viewer-d3fc/src/js/charts/ohlc.js
@@ -15,6 +15,7 @@ ohlc.plugin = {
     name: "OHLC Chart",
     max_cells: 3500,
     max_columns: 50,
+    render_warning: true,
     initial: {
         type: "number",
         count: 4,

diff --git a/packages/perspective-viewer-d3fc/src/js/charts/sunburst.js b/packages/perspective-viewer-d3fc/src/js/charts/sunburst.js
@@ -76,6 +76,7 @@ sunburst.plugin = {
     name: "Sunburst",
     max_cells: 7500,
     max_columns: 50,
+    render_warning: true,
     initial: {
         type: "number",
         count: 1,

diff --git a/packages/perspective-viewer-d3fc/src/js/charts/treemap.js b/packages/perspective-viewer-d3fc/src/js/charts/treemap.js
@@ -71,6 +71,7 @@ treemap.plugin = {
     name: "Treemap",
     max_cells: 5000,
     max_columns: 50,
+    render_warning: true,
     initial: {
         type: "number",
         count: 1,

diff --git a/packages/perspective-viewer-d3fc/src/js/charts/xy-line.js b/packages/perspective-viewer-d3fc/src/js/charts/xy-line.js
@@ -96,6 +96,7 @@ xyLine.plugin = {
     name: "X/Y Line Chart",
     max_cells: 50000,
     max_columns: 50,
+    render_warning: true,
     initial: {
         type: "number",
         count: 2,

diff --git a/packages/perspective-viewer-d3fc/src/js/charts/xy-scatter.js b/packages/perspective-viewer-d3fc/src/js/charts/xy-scatter.js
@@ -98,6 +98,7 @@ xyScatter.plugin = {
     name: "X/Y Scatter Chart",
     max_cells: 50000,
     max_columns: 50,
+    render_warning: true,
     initial: {
         type: "number",
         count: 2,

diff --git a/packages/perspective-viewer-d3fc/src/js/charts/y-scatter.js b/packages/perspective-viewer-d3fc/src/js/charts/y-scatter.js
@@ -96,7 +96,8 @@ yScatter.plugin = {
     type: "d3_y_scatter",
     name: "Y Scatter Chart",
     max_cells: 4000,
-    max_columns: 50
+    max_columns: 50,
+    render_warning: true
 };
 
 export default yScatter;
diff --git a/packages/perspective-viewer-d3fc/test/js/integration/bar.spec.js b/packages/perspective-viewer-d3fc/test/js/integration/bar.spec.js
@@ -11,6 +11,7 @@ const path = require("path");
 
 const utils = require("@finos/perspective-test");
 const simple_tests = require("@finos/perspective-viewer/test/js/simple_tests.js");
+const render_warning_tests = require("@finos/perspective-viewer/test/js/render_warning_tests.js");
 
 const {withTemplate} = require("./simple-template");
 
@@ -23,6 +24,7 @@ utils.with_server({}, () => {
         "bar.html",
         () => {
             simple_tests.default();
+            render_warning_tests.default("d3_y_bar");
         },
         {root: path.join(__dirname, "..", "..", "..")}
     );
@@ -31,6 +33,7 @@ utils.with_server({}, () => {
         "bar-x.html",
         () => {
             simple_tests.default();
+            render_warning_tests.default("d3_x_bar");
         },
         {root: path.join(__dirname, "..", "..", "..")}
     );
@@ -39,6 +42,7 @@ utils.with_server({}, () => {
         "bar-themed.html",
         () => {
             simple_tests.default();
+            render_warning_tests.default("d3_y_bar");
         },
         {root: path.join(__dirname, "..", "..", "..")}
     );

diff --git a/packages/perspective-viewer-d3fc/test/js/integration/heatmap.spec.js b/packages/perspective-viewer-d3fc/test/js/integration/heatmap.spec.js
@@ -11,6 +11,7 @@ const path = require("path");
 
 const utils = require("@finos/perspective-test");
 const simple_tests = require("@finos/perspective-viewer/test/js/simple_tests.js");
+const render_warning_tests = require("@finos/perspective-viewer/test/js/render_warning_tests.js");
 
 const {withTemplate} = require("./simple-template");
 withTemplate("heatmap", "d3_heatmap");
@@ -20,6 +21,7 @@ utils.with_server({}, () => {
         "heatmap.html",
         () => {
             simple_tests.default();
+            render_warning_tests.default("d3_heatmap");
         },
         {reload_page: false, root: path.join(__dirname, "..", "..", "..")}
     );

diff --git a/packages/perspective-viewer-d3fc/test/js/integration/yscatter.spec.js b/packages/perspective-viewer-d3fc/test/js/integration/yscatter.spec.js
@@ -11,6 +11,7 @@ const path = require("path");
 
 const utils = require("@finos/perspective-test");
 const simple_tests = require("@finos/perspective-viewer/test/js/simple_tests.js");
+const render_warning_tests = require("@finos/perspective-viewer/test/js/render_warning_tests.js");
 
 const {withTemplate} = require("./simple-template");
 withTemplate("yscatter", "d3_y_scatter");
@@ -20,6 +21,7 @@ utils.with_server({}, () => {
         "yscatter.html",
         () => {
             simple_tests.default();
+            render_warning_tests.default("d3_y_scatter");
         },
         {reload_page: false, root: path.join(__dirname, "..", "..", "..")}
     );

diff --git a/packages/perspective-viewer-d3fc/test/results/linux.docker.json b/packages/perspective-viewer-d3fc/test/results/linux.docker.json
@@ -1,8 +1,8 @@
 {
-    "candlestick_filter_by_a_single_instrument_": "f988ca6494d7a36bada09928cd1a544e",
+    "candlestick_filter_by_a_single_instrument_": "d3ef6be674cc045b4c8551f111281f78",
     "candlestick_filter_to_date_range_": "a08755a828b3ab52426cb306c04ad0d7",
-    "__GIT_COMMIT__": "ffcc3be9fb137f23a21e3737e07e809e55e71e49",
-    "ohlc_filter_by_a_single_instrument_": "0110fac1f2befac1b97a9d33f0022acf",
+    "__GIT_COMMIT__": "6ebb74ab5cc1d3d5bdcf127ec0db8452f1987a45",
+    "ohlc_filter_by_a_single_instrument_": "ed4232a783fbd733ce9267ddda98b7f0",
     "ohlc_filter_to_date_range_": "96db579b0cb0fa173ed7e6d4d9539fa2",
     "scatter_shows_a_grid_without_any_settings_applied_": "8677946ab48f16a376c421500d59e6c0",
     "scatter_pivots_by_a_row_": "a4dd9b7daff4e639fb7a56b01d2f4990",
@@ -141,5 +141,35 @@
     "xyline_filters_by_a_datetime_column_": "686bf3d9a7ae981505826700d023a61a",
     "xyline_highlights_invalid_filter_": "824c12f472d7df047c4c221d0168dadd",
     "xyline_sorts_by_an_alpha_column_": "e2410b6def6a5916e9a9f96063ad1352",
-    "xyline_displays_visible_columns_": "dcd473a0a6f0dfe49b8a0b8781757a7c"
+    "xyline_displays_visible_columns_": "dcd473a0a6f0dfe49b8a0b8781757a7c",
+    "bar_warning_should_be_shown_when_points_exceed_max_cells_and_max_columns": "1f9c492fe058350dfd07c955e4c3182d",
+    "bar_warning_should_be_shown_when_points_exceed_max_cells_but_not_max_columns": "f0cf3b07f826f52d95bc9050b52c64db",
+    "bar_warning_should_be_shown_when_points_exceed_max_columns_but_not_max_cells": "801cbe18dab86abbd93b35491cc80080",
+    "bar_warning_should_be_re-rendered_if_the_config_is_changed_and_points_exceed_max": "801cbe18dab86abbd93b35491cc80080",
+    "bar_warning_should_not_be_re-rendered_if_the_config_is_changed_and_points_do_not_exceed_max": "c123d621f5f28437c8698fbe9c384728",
+    "bar_warning_should_not_be_rendered_again_after_the_user_clicks_render_all_points": "1f05896897d8aa4ba091c333e6102bbf",
+    "bar-x_warning_should_be_shown_when_points_exceed_max_cells_and_max_columns": "880d3b5ad6a6aa9256c9569aece5ac67",
+    "bar-x_warning_should_be_shown_when_points_exceed_max_cells_but_not_max_columns": "e3e66240f40cc1a57b65264ab42ab794",
+    "bar-x_warning_should_be_shown_when_points_exceed_max_columns_but_not_max_cells": "325bcee386469256fbb38f0114aa1601",
+    "bar-x_warning_should_be_re-rendered_if_the_config_is_changed_and_points_exceed_max": "325bcee386469256fbb38f0114aa1601",
+    "bar-x_warning_should_not_be_re-rendered_if_the_config_is_changed_and_points_do_not_exceed_max": "63eca7730c561650c219a4c67ffd9674",
+    "bar-x_warning_should_not_be_rendered_again_after_the_user_clicks_render_all_points": "e59d30068e277518527c19fe5129be94",
+    "bar-themed_warning_should_be_shown_when_points_exceed_max_cells_and_max_columns": "6d5bf3c131e9f648e571305d699902d7",
+    "bar-themed_warning_should_be_shown_when_points_exceed_max_cells_but_not_max_columns": "5a964018d085a0aa08729589f7f7fcda",
+    "bar-themed_warning_should_be_shown_when_points_exceed_max_columns_but_not_max_cells": "3884a75cb5fba2cc74e704a29ed358e1",
+    "bar-themed_warning_should_be_re-rendered_if_the_config_is_changed_and_points_exceed_max": "3884a75cb5fba2cc74e704a29ed358e1",
+    "bar-themed_warning_should_not_be_re-rendered_if_the_config_is_changed_and_points_do_not_exceed_max": "fc1131fe4cc6d7c79ebc4b54bf557d14",
+    "bar-themed_warning_should_not_be_rendered_again_after_the_user_clicks_render_all_points": "c639aab001f30196eca528bb81798289",
+    "yscatter_warning_should_be_shown_when_points_exceed_max_cells_and_max_columns": "8b52a17adc1b2c24ec31a114c0ea93ce",
+    "yscatter_warning_should_be_shown_when_points_exceed_max_cells_but_not_max_columns": "8b52a17adc1b2c24ec31a114c0ea93ce",
+    "yscatter_warning_should_be_shown_when_points_exceed_max_columns_but_not_max_cells": "8b52a17adc1b2c24ec31a114c0ea93ce",
+    "yscatter_warning_should_be_re-rendered_if_the_config_is_changed_and_points_exceed_max": "8b52a17adc1b2c24ec31a114c0ea93ce",
+    "yscatter_warning_should_not_be_re-rendered_if_the_config_is_changed_and_points_do_not_exceed_max": "5012831355ddad931dbb17862e45cf88",
+    "yscatter_warning_should_not_be_rendered_again_after_the_user_clicks_render_all_points": "d381f4f93ea11078867af537ac5059b0",
+    "heatmap_warning_should_be_shown_when_points_exceed_max_cells_and_max_columns": "0aeaf77412802ee8d52af2d9a6c12ff1",
+    "heatmap_warning_should_be_shown_when_points_exceed_max_cells_but_not_max_columns": "0aeaf77412802ee8d52af2d9a6c12ff1",
+    "heatmap_warning_should_be_shown_when_points_exceed_max_columns_but_not_max_cells": "0aeaf77412802ee8d52af2d9a6c12ff1",
+    "heatmap_warning_should_be_re-rendered_if_the_config_is_changed_and_points_exceed_max": "0aeaf77412802ee8d52af2d9a6c12ff1",
+    "heatmap_warning_should_not_be_re-rendered_if_the_config_is_changed_and_points_do_not_exceed_max": "065407709ce1894d5d2fd55d17f0c03d",
+    "heatmap_warning_should_not_be_rendered_again_after_the_user_clicks_render_all_points": "538683db8516c434100ac6ec14ef1484"
 }
diff --git a/packages/perspective-viewer/src/html/viewer.html b/packages/perspective-viewer/src/html/viewer.html
@@ -73,7 +73,6 @@
                     <span class="plugin_information__text" id="plugin_information_count">Estimated rendering</span>
                     <span class="plugin_information__actions">
                         <span class="plugin_information__action">Render all points</span>
-                        <span class="plugin_information__action--close" id="close_button"></span>
                     </span>
                 </div>
                 <div id="pivot_chart"></div>

diff --git a/packages/perspective-viewer/src/js/viewer/action_element.js b/packages/perspective-viewer/src/js/viewer/action_element.js
@@ -369,10 +369,7 @@ export class ActionElement extends DomElement {
         this._plugin_information_action.addEventListener("click", () => {
             this._debounce_update({ignore_size_check: true, limit_points: false});
             this._plugin_information.classList.add("hidden");
-        });
-
-        this._plugin_information_action_close.addEventListener("click", () => {
-            this._plugin_information.classList.add("hidden");
+            this._plugin.render_warning = false;
         });
     }
 }
diff --git a/packages/perspective-viewer/src/js/viewer/dom_element.js b/packages/perspective-viewer/src/js/viewer/dom_element.js
@@ -494,7 +494,6 @@ export class DomElement extends PerspectiveElement {
         this._transpose_button = this.shadowRoot.querySelector("#transpose_button");
         this._plugin_information = this.shadowRoot.querySelector(".plugin_information");
         this._plugin_information_action = this.shadowRoot.querySelector(".plugin_information__action");
-        this._plugin_information_action_close = this.shadowRoot.querySelector(".plugin_information__action--close");
         this._plugin_information_message = this.shadowRoot.querySelector("#plugin_information_count");
         this._resize_bar = this.shadowRoot.querySelector("#resize_bar");
     }

diff --git a/packages/perspective-viewer/src/js/viewer/perspective_element.js b/packages/perspective-viewer/src/js/viewer/perspective_element.js
@@ -300,6 +300,15 @@ export class PerspectiveElement extends StateElement {
     }
 
     async get_maxes() {
+        // If the plugin is set to not render a warning, i.e. after the user
+        // selects "Render all points", then return null for max_cols/max_rows.
+        if (typeof this._plugin.max_columns !== "undefined" && this._plugin.render_warning === false) {
+            return {
+                max_cols: null,
+                max_rows: null
+            };
+        }
+
         let max_cols, max_rows;
         const [schema, num_columns] = await Promise.all([this._view.schema(), this._view.num_columns()]);
         const schema_columns = Object.keys(schema || {}).length || 1;

diff --git a/packages/perspective-viewer/src/less/default.less b/packages/perspective-viewer/src/less/default.less
@@ -368,13 +368,6 @@
             margin-right: 0.25rem;
         }
 
-        .plugin_information__action--close {
-            margin-left: 12px;
-            display: inline-flex;
-            font-size: 12px;
-            cursor: pointer;
-        }
-
         .plugin_information__actions {
             margin-left: auto;
             display: flex;

diff --git a/packages/perspective-viewer/test/js/render_warning_tests.js b/packages/perspective-viewer/test/js/render_warning_tests.js
@@ -0,0 +1,106 @@
+/******************************************************************************
+ *
+ * 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.
+ *
+ */
+
+exports.default = function(plugin_name, columns) {
+    let view_columns = ["Sales"];
+
+    if (columns) {
+        // Handle cases where multiple columns are required for a proper
+        // chart render, i.e. in scatter charts where there is an X and Y axis.
+        view_columns = columns;
+    }
+
+    view_columns = JSON.stringify(view_columns);
+
+    test.capture("warning should be shown when points exceed max_cells and max_columns", async page => {
+        const viewer = await page.$("perspective-viewer");
+        await page.evaluate(plugin_name => {
+            const plugin = window.getPlugin(plugin_name);
+            plugin.max_columns = 1;
+            plugin.max_cells = 1;
+        }, plugin_name);
+
+        await page.shadow_click("perspective-viewer", "#config_button");
+        await page.evaluate((element, view_columns) => element.setAttribute("columns", view_columns), viewer, view_columns);
+        await page.evaluate(element => element.setAttribute("column-pivots", '["Profit"]'), viewer);
+        await page.waitForSelector("perspective-viewer:not([updating])");
+    });
+
+    test.capture("warning should be shown when points exceed max_cells but not max_columns", async page => {
+        const viewer = await page.$("perspective-viewer");
+        await page.evaluate(plugin_name => {
+            const plugin = window.getPlugin(plugin_name);
+            plugin.max_cells = 1;
+        }, plugin_name);
+
+        await page.shadow_click("perspective-viewer", "#config_button");
+        await page.evaluate((element, view_columns) => element.setAttribute("columns", view_columns), viewer, view_columns);
+        await page.evaluate(element => element.setAttribute("column-pivots", '["Profit"]'), viewer);
+        await page.waitForSelector("perspective-viewer:not([updating])");
+    });
+
+    test.capture("warning should be shown when points exceed max_columns but not max_cells", async page => {
+        const viewer = await page.$("perspective-viewer");
+        await page.evaluate(plugin_name => {
+            const plugin = window.getPlugin(plugin_name);
+            plugin.max_columns = 1;
+        }, plugin_name);
+
+        await page.shadow_click("perspective-viewer", "#config_button");
+        await page.evaluate((element, view_columns) => element.setAttribute("columns", view_columns), viewer, view_columns);
+        await page.evaluate(element => element.setAttribute("column-pivots", '["Profit"]'), viewer);
+        await page.waitForSelector("perspective-viewer:not([updating])");
+    });
+
+    test.capture("warning should be re-rendered if the config is changed and points exceed max", async page => {
+        const viewer = await page.$("perspective-viewer");
+        await page.evaluate(plugin_name => {
+            const plugin = window.getPlugin(plugin_name);
+            plugin.max_columns = 1;
+        }, plugin_name);
+
+        await page.shadow_click("perspective-viewer", "#config_button");
+        await page.evaluate((element, view_columns) => element.setAttribute("columns", view_columns), viewer, view_columns);
+        await page.evaluate(element => element.setAttribute("column-pivots", '["Row ID"]'), viewer);
+        await page.waitForSelector("perspective-viewer:not([updating])");
+        await page.evaluate(element => element.setAttribute("column-pivots", '["Profit"]'), viewer);
+        await page.waitForSelector("perspective-viewer:not([updating])");
+    });
+
+    test.capture("warning should not be re-rendered if the config is changed and points do not exceed max", async page => {
+        const viewer = await page.$("perspective-viewer");
+        await page.evaluate(plugin_name => {
+            const plugin = window.getPlugin(plugin_name);
+            plugin.max_columns = 5;
+        }, plugin_name);
+
+        await page.shadow_click("perspective-viewer", "#config_button");
+        await page.evaluate((element, view_columns) => element.setAttribute("columns", view_columns), viewer, view_columns);
+        await page.evaluate(element => element.setAttribute("column-pivots", '["Profit"]'), viewer);
+        await page.waitForSelector("perspective-viewer:not([updating])");
+        await page.evaluate(element => element.removeAttribute("column-pivots"), viewer);
+        await page.waitForSelector("perspective-viewer:not([updating])");
+    });
+
+    test.capture("warning should not be rendered again after the user clicks render all points", async page => {
+        const viewer = await page.$("perspective-viewer");
+        await page.evaluate(plugin_name => {
+            const plugin = window.getPlugin(plugin_name);
+            plugin.max_columns = 1;
+        }, plugin_name);
+
+        await page.shadow_click("perspective-viewer", "#config_button");
+        await page.evaluate((element, view_columns) => element.setAttribute("columns", view_columns), viewer, view_columns);
+        await page.evaluate(element => element.setAttribute("column-pivots", '["Row ID"]'), viewer);
+        await page.shadow_click("perspective-viewer", ".plugin_information__action");
+        await page.waitForSelector("perspective-viewer:not([updating])");
+        await page.evaluate(element => element.setAttribute("column-pivots", '["Profit"]'), viewer);
+        await page.waitForSelector("perspective-viewer:not([updating])");
+    });
+};