Skip to content

Improve CONTRIBUTING.md #4624

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Mar 12, 2020
81 changes: 75 additions & 6 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -96,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 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
Expand Down Expand Up @@ -175,11 +183,23 @@ 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 `<ctrl-shift-j>`)
- Find test file (e.g. with `<ctrl-o>` + 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 `<crtl-r>`)

![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:

```
```js
Plotly.newPlot(gd, [{
x: [1, 1, 1, 2, 2, 2, 3, 3, 3],
y: [1, 2, 3, 1, 2, 3, 1, 2, 3],
Expand All @@ -196,18 +216,67 @@ This will produce the following plot, and say you want to simulate a selection p

<img src="https://user-images.githubusercontent.com/31989842/38890553-0bc6190c-4282-11e8-8efc-077bf05ca565.png">


## Repo organization

- 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`)

## 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 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 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 `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` (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. 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.
- `_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 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
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 make the logic easier to test.
If you make a `convert`, you should call it from either `calc` or `plot`.

## Coding style

Expand Down
5 changes: 4 additions & 1 deletion test/image/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down