From 0a63621194e2316c23500e9141bfb02cc7b453f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 11 Mar 2020 10:16:38 -0400 Subject: [PATCH 01/10] add info about `new trace type` label --- CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 16973c49bba..844e504dee4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -17,6 +17,7 @@ We use the following [labels](https://github.com/plotly/plotly.js/labels) to tra | `type: bug` | bug report confirmed by a plotly team member | | `type: regression` | bug that introduced a change in behavior from one version to the next | | `type: feature` | planned feature additions | +| `type: new trace type` | subset of `type: feature` reserved for planned new trace types | | `type: translation` | localization-related tasks | | `type: performance` | performance related tasks | | `type: maintenance` | source code cleanup resulting in no enhancement for users | From f6ce11109b44b2095da657491d1936a6ca4e7fbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 11 Mar 2020 10:16:54 -0400 Subject: [PATCH 02/10] write info about `plotly-mock-viewer` --- CONTRIBUTING.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 844e504dee4..1c0e56c2e02 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -97,6 +97,13 @@ Three additional helpers exist that are refreshed every second: There is also a search bar in the top right of the dashboard. This fuzzy-searches image mocks based on their file name and trace type. +#### Alternative to test dashboard + +Use the [`plotly-mock-viewer`](https://github.com/rreusser/plotly-mock-viewer) +which has live-reloading and a bunch of other cool features. +An online version of `plotly-mock-viewer` is available at https://rreusser.github.io/plotly-mock-viewer/ +which uses the https://cdn.plot.ly/plotly-latest.min.js + #### Other npm scripts - `npm run preprocess`: pre-processes the css and svg source file in js. This From d2df81ecaaa8867e82181757f3b77ab616565d7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 11 Mar 2020 10:34:16 -0400 Subject: [PATCH 03/10] add one more example of `npm run test-image` --- test/image/README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/image/README.md b/test/image/README.md index 16ebfbfa881..0b9e5f18d13 100644 --- a/test/image/README.md +++ b/test/image/README.md @@ -93,8 +93,11 @@ For example, # Run one test (e.g. the 'contour_nolines' test): $ npm run test-image -- contour_nolines -# Run all gl3d image test in batch: +# Run all gl3d image tests in batch: $ npm run test-image -- gl3d_* + +# Run all image tests that are not gl3d in batch: +npm run test-image -- "\!\(gl3d_\)*" ``` Developers on weak hardware might encounter batch timeout issue. These are most From 202aab7ce4ef1c39d2a33daf6a5f967d6ea113ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 11 Mar 2020 11:01:59 -0400 Subject: [PATCH 04/10] minor fixups in "Writing interaction tests" --- CONTRIBUTING.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1c0e56c2e02..392efa753c8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -184,10 +184,11 @@ which shows the baseline image, the generated image, the diff and the json mocks To view the results of a run on CircleCI, download the `build/test_images/` and `build/test_images_diff/` artifacts into your local repo and then run `npm run start-image_viewer`. ### Writing interaction tests + Keep in mind that the interaction coordinates are relative to the top-left corner of the plot, including the margins. To produce a reliable interaction test, it may be necessary to fix the width, height, margins, X axis range and Y axis range of the plot. For example: -``` +```js Plotly.newPlot(gd, [{ x: [1, 1, 1, 2, 2, 2, 3, 3, 3], y: [1, 2, 3, 1, 2, 3, 1, 2, 3], From ebfa157cb7b3644e4087c350795e2f12f0be5508 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 11 Mar 2020 11:05:42 -0400 Subject: [PATCH 05/10] add "Using the developer console in karma" section --- CONTRIBUTING.md | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 392efa753c8..4d0309721f5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -183,7 +183,18 @@ which shows the baseline image, the generated image, the diff and the json mocks To view the results of a run on CircleCI, download the `build/test_images/` and `build/test_images_diff/` artifacts into your local repo and then run `npm run start-image_viewer`. -### Writing interaction tests +### Using the developer console in karma to write/debug jasmine tests + +- Click on the `DEBUG` button +- In the `DEBUG RUNNER` window, open the console (e.g. with ``) +- Find test file (e.g. with `` + typing the name of the file), look out + for "bundled" files with the same name. +- Set `debugger` on relevant line(s) +- Rerun the test suite by refreshing the page (e.g. with ``) + +![Peek 2020-03-11 10-45](https://user-images.githubusercontent.com/6675409/76438118-f2502300-6390-11ea-88d2-17a553c3b4e8.gif) + +### Writing jasmine interaction tests Keep in mind that the interaction coordinates are relative to the top-left corner of the plot, including the margins. To produce a reliable interaction test, it may be necessary to fix the width, height, margins, X axis range and Y axis range of the plot. For example: @@ -205,7 +216,6 @@ This will produce the following plot, and say you want to simulate a selection p - ## Repo organization - Distributed files are in `dist/` From 636b44e5f1c19fd36e8d8b9e9b66b49e9f5ab609 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 11 Mar 2020 12:00:46 -0400 Subject: [PATCH 06/10] update the "Repo organization" section --- CONTRIBUTING.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4d0309721f5..fdbe6e87b06 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -220,13 +220,12 @@ This will produce the following plot, and say you want to simulate a selection p - Distributed files are in `dist/` - CommonJS require-able modules are in `lib/` -- Sources files are in `src/`, including the index +- Sources files are in `src/` - Build and repo management scripts are in `tasks/` - All tasks can be run using [`npm run-script`](https://docs.npmjs.com/cli/run-script) - Tests are `test/`, they are partitioned into `image` and `jasmine` tests - Test dashboard and image viewer code is in `devtools/` -- Built files are in `build/` (most files in here are git-ignored, the css and font built files are exceptions) - +- Built files are in `build/` (the files in here are git-ignored, except for `plotcss.js`) ## Coding style From 278ed7cf71304907cef2c4593e4430d82760f685 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 11 Mar 2020 12:04:37 -0400 Subject: [PATCH 07/10] first cut "Trace module design" section --- CONTRIBUTING.md | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fdbe6e87b06..94fada232fa 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -227,6 +227,55 @@ This will produce the following plot, and say you want to simulate a selection p - Test dashboard and image viewer code is in `devtools/` - Built files are in `build/` (the files in here are git-ignored, except for `plotcss.js`) +## Trace module design + +The trace modules (found in [`src/traces`](https://github.com/plotly/plotly.js/tree/master/src/traces)) +are defined as plain objects with functions and constants attached to them in an index file +(e.g. `src/traces/scatter/index.js`). The trace modules are "registered" undo the `Registry` object +(found in [`src/registry.js`](https://github.com/plotly/plotly.js/blob/master/src/registry.js)) using +`Plotly.register` (as done in the index files in `dist/`). + +The trace module methods are meant to be called as part of loops during subplot-specific +(e.g. in `plots/cartesian/index.js`) and figure-wide (e.g. in `plots/plots.js`) subroutines. +That way, the subroutines work no matter which trace modules got registered. + +All traces modules defined: + +- `_module.name`: name of the trace module as used by the trace `type` attribute. +- `_module.basePlotModule`: base plot (or subplot) module corresponding to the + trace type (e.g. `scatter` links up the `Cartesian` base plot module, `scatter3d` links up `gl3d`). +- `_module.attributes`: JSON-serializable object of attribute declarations. + This object is used to generate the plot-schema JSON. +- `_module.supplyDefaults`: Takes in input trace settings and coerces them into "full" settings + under `gd._fullData`. This one is called during the figure-wide `Plots.supplyDefaults` routine. + Note that the `suppyDefaults` method performance should scale with the number of attribute **not** the + number of data points. +- `_module.calc`: Converts inputs data into "calculated" (or sanitized) data. This one is called during + the figure-wide `Plots.doCalcdata` routine. The `calc` method is allowed to + scale with the number of data points and is in general more costly than `supplyDefaults`. + Please note that some edit pathways skip `Plots.doCalcdata`. +- `_module.plot`: Draws the trace on screen. This one is called by the defined `basePlotModule`. + +Other methods used by some trace modules: + +- `_module.categories`: list of string identifies used to grouped traces by behavior +- `_module.layoutAttributes`: JSON-serializable object of attribute declarations + in the layout (e.g. `barmode` for `bar` traces) +- `_module.supplyLayoutDefaults`: Default logic for layout attributes. +- `_module.crossTraceDefaults`: Default logic that depends on input setting of multiple traces. +- `_module.crossTraceCalc`: Computations that depend on the data of multiple traces. +- `_module.colorbar`: Defines the colorbar appearance when the trace supports it. +- `_module.hoverPoints`: Point-picking logic called during hover. +- `_module.selectPoints`: Polygon-containing logic called during selections. +- `_module.style`: Sometimes split from `_module.plot` where `_module.plot` only + draws the elements and `_module.style` styles them. +- `_module.styleOnSelect`: Optimization of `_module.style` called during + selections. +- `_module.convert`: Sometimes separated from `_module.plot` or `_module.calc` to convert the + plotly.js settings to another framework e.g. to `gl-plot3d` for `gl3d` traces, to + `mapbox-gl` from `mapbox` traces. This split can also make the logic easier to + test. + ## Coding style Check if ok, with `npm run lint` From 7487dc32157b40c48bfc2d15ac9d8cba20bbcb17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 11 Mar 2020 12:19:12 -0400 Subject: [PATCH 08/10] typo fixes --- CONTRIBUTING.md | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 94fada232fa..fe0732aefd8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -102,7 +102,7 @@ image mocks based on their file name and trace type. Use the [`plotly-mock-viewer`](https://github.com/rreusser/plotly-mock-viewer) which has live-reloading and a bunch of other cool features. An online version of `plotly-mock-viewer` is available at https://rreusser.github.io/plotly-mock-viewer/ -which uses the https://cdn.plot.ly/plotly-latest.min.js +which uses https://cdn.plot.ly/plotly-latest.min.js #### Other npm scripts @@ -239,7 +239,7 @@ The trace module methods are meant to be called as part of loops during subplot- (e.g. in `plots/cartesian/index.js`) and figure-wide (e.g. in `plots/plots.js`) subroutines. That way, the subroutines work no matter which trace modules got registered. -All traces modules defined: +All traces modules set: - `_module.name`: name of the trace module as used by the trace `type` attribute. - `_module.basePlotModule`: base plot (or subplot) module corresponding to the @@ -248,8 +248,8 @@ All traces modules defined: This object is used to generate the plot-schema JSON. - `_module.supplyDefaults`: Takes in input trace settings and coerces them into "full" settings under `gd._fullData`. This one is called during the figure-wide `Plots.supplyDefaults` routine. - Note that the `suppyDefaults` method performance should scale with the number of attribute **not** the - number of data points. + Note that the `suppyDefaults` method performance should scale with the number of attributes (**not** the + number of data points). - `_module.calc`: Converts inputs data into "calculated" (or sanitized) data. This one is called during the figure-wide `Plots.doCalcdata` routine. The `calc` method is allowed to scale with the number of data points and is in general more costly than `supplyDefaults`. @@ -258,13 +258,13 @@ All traces modules defined: Other methods used by some trace modules: -- `_module.categories`: list of string identifies used to grouped traces by behavior +- `_module.categories`: list of string identifiers used to group traces by behavior - `_module.layoutAttributes`: JSON-serializable object of attribute declarations - in the layout (e.g. `barmode` for `bar` traces) -- `_module.supplyLayoutDefaults`: Default logic for layout attributes. -- `_module.crossTraceDefaults`: Default logic that depends on input setting of multiple traces. + coerced in the layout (e.g. `barmode` for `bar` traces) +- `_module.supplyLayoutDefaults`: Defaults logic for layout attributes. +- `_module.crossTraceDefaults`: Defaults logic that depends on input setting of multiple traces. - `_module.crossTraceCalc`: Computations that depend on the data of multiple traces. -- `_module.colorbar`: Defines the colorbar appearance when the trace supports it. +- `_module.colorbar`: Defines the colorbar appearance for traces that support it. - `_module.hoverPoints`: Point-picking logic called during hover. - `_module.selectPoints`: Polygon-containing logic called during selections. - `_module.style`: Sometimes split from `_module.plot` where `_module.plot` only @@ -273,8 +273,7 @@ Other methods used by some trace modules: selections. - `_module.convert`: Sometimes separated from `_module.plot` or `_module.calc` to convert the plotly.js settings to another framework e.g. to `gl-plot3d` for `gl3d` traces, to - `mapbox-gl` from `mapbox` traces. This split can also make the logic easier to - test. + `mapbox-gl` from `mapbox` traces. This split can make the logic easier to test. ## Coding style From 8daa1f1b2c41af3942f6d4bcc2a16de1e5d7ee46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 11 Mar 2020 17:55:24 -0400 Subject: [PATCH 09/10] aj-proof improvements - fix a couple typos - make `supplyDefaults` not looping over data arrays more clear - mention `editType` in comment about `Plots.doCalcdata` getting skipped - mention `Registry.traceIs` in item about module categories --- CONTRIBUTING.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fe0732aefd8..a68702313e9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -243,22 +243,24 @@ All traces modules set: - `_module.name`: name of the trace module as used by the trace `type` attribute. - `_module.basePlotModule`: base plot (or subplot) module corresponding to the - trace type (e.g. `scatter` links up the `Cartesian` base plot module, `scatter3d` links up `gl3d`). + trace type (e.g. `scatter` links to the `Cartesian` base plot module, `scatter3d` links to `gl3d`). - `_module.attributes`: JSON-serializable object of attribute declarations. This object is used to generate the plot-schema JSON. - `_module.supplyDefaults`: Takes in input trace settings and coerces them into "full" settings under `gd._fullData`. This one is called during the figure-wide `Plots.supplyDefaults` routine. - Note that the `suppyDefaults` method performance should scale with the number of attributes (**not** the - number of data points). + Note that the `supplyDefaults` method performance should scale with the number of attributes (**not** the + number of data points - so it should not loop over any data arrays). - `_module.calc`: Converts inputs data into "calculated" (or sanitized) data. This one is called during the figure-wide `Plots.doCalcdata` routine. The `calc` method is allowed to scale with the number of data points and is in general more costly than `supplyDefaults`. - Please note that some edit pathways skip `Plots.doCalcdata`. + Please note that some edit pathways skip `Plots.doCalcdata` (as determined by the + `editType` flags in the attributes files). - `_module.plot`: Draws the trace on screen. This one is called by the defined `basePlotModule`. Other methods used by some trace modules: -- `_module.categories`: list of string identifiers used to group traces by behavior +- `_module.categories`: list of string identifiers used to group traces by behavior. Traces that + have a given category can then be detected using [`Registry.traceIs`](https://github.com/plotly/plotly.js/blob/8f049fddbac0ca0382816984b8526857e9714fe6/src/registry.js#L129-L155) - `_module.layoutAttributes`: JSON-serializable object of attribute declarations coerced in the layout (e.g. `barmode` for `bar` traces) - `_module.supplyLayoutDefaults`: Defaults logic for layout attributes. From 2cc7eb3b76cbf742d6f8bf1a92163decf1a871e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 12 Mar 2020 09:13:45 -0400 Subject: [PATCH 10/10] make `_module.convert` more clear. Co-Authored-By: alexcjohnson --- CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a68702313e9..aec1580e6a9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -276,6 +276,7 @@ Other methods used by some trace modules: - `_module.convert`: Sometimes separated from `_module.plot` or `_module.calc` to convert the plotly.js settings to another framework e.g. to `gl-plot3d` for `gl3d` traces, to `mapbox-gl` from `mapbox` traces. This split can make the logic easier to test. + If you make a `convert`, you should call it from either `calc` or `plot`. ## Coding style