Skip to content
This repository was archived by the owner on Feb 10, 2025. It is now read-only.

Async canvas component / IE11 compatibility #29

Merged
merged 21 commits into from
Nov 19, 2019
Merged

Conversation

Marc-Andre-Rivet
Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet commented Oct 24, 2019

Followup to plotly/dash#973

Now related to plotly/dash#1006

@Marc-Andre-Rivet Marc-Andre-Rivet added this to the Dash v1.7 milestone Nov 5, 2019
@Marc-Andre-Rivet Marc-Andre-Rivet changed the title [WIP] Async canvas component Async canvas component Nov 5, 2019
@Marc-Andre-Rivet Marc-Andre-Rivet marked this pull request as ready for review November 5, 2019 21:31
"@babel/polyfill": "^7.6.0",
"@babel/preset-env": "^7.6.2",
"@babel/preset-react": "^7.0.0",
"@plotly/webpack-dash-dynamic-import": "^1.1.2",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the dash custom dynamic import plugin / also, updating the toolchain

@@ -18,7 +18,7 @@
"build:js-dev": "webpack --mode development",
"build:js": "webpack --mode production",
"build:py": "node ./extract-meta.js src/lib/components > dash_canvas/metadata.json && copyfiles package.json dash_canvas && venv/bin/python -c \"import dash; dash.development.component_loader.generate_classes('dash_canvas', 'dash_canvas/metadata.json')\"",
"build:all": "npm run build:js & npm run build:js-dev & npm run build:py"
"build:all": "npm run build:js && npm run build:js-dev && npm run build:py"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

& makes build:py at the same time as the build.. we want them in sequence

setup.py Outdated
@@ -19,6 +19,6 @@
include_package_data=True,
license=package['license'],
description=package['description'] if 'description' in package else package_name,
install_requires=['dash', 'dash-html-components', 'scikit-image', 'Pillow',
install_requires=['dash>=1.6.0', 'dash-html-components', 'scikit-image', 'Pillow',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now needs a version of dash with async support

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we take html, dcc, and plotly out of this list since they all come in with dash?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dash-daq
ipdb
percy
selenium
flake8
pylint
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

most of these are now covered by dash's test environment

Co-Authored-By: alexcjohnson <[email protected]>

DashCanvas.propTypes = {
/**
* The ID used to identify this component in Dash callbacks
*/
id: PropTypes.string,
id: PropTypes.string,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\t?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

weird..

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there seems to be a mishmash of whitespace characters in various files...

@alexcjohnson
Copy link

LGTM! @emmanuelle I'll let you give final approval, just in case there's anything unusual - derived props or environment requirements... or if you have comments on any of the build changes here.

Marc-André Rivet added 2 commits November 5, 2019 17:47
Marc-André Rivet added 2 commits November 7, 2019 10:51
@Marc-Andre-Rivet Marc-Andre-Rivet changed the title Async canvas component Async canvas component / IE11 compatibility Nov 8, 2019
@alexcjohnson
Copy link

just a few new comments #29 (comment) #29 (comment) and there are still \t in several src files #29 (comment) - but these are all pretty minor concerns, overall looks great. Would like to have @emmanuelle take a look too though.

@Marc-Andre-Rivet
Copy link
Contributor Author

Here's the docs with the candidate version: https://github.com/plotly/dash-docs/pull/699

Percy diffs were all related to (1) version change, (2) new arrow button

@emmanuelle
Copy link
Contributor

Thank you @Marc-Andre-Rivet it looks good to me! 💃

@Marc-Andre-Rivet Marc-Andre-Rivet merged commit e9170b1 into master Nov 19, 2019
@Marc-Andre-Rivet Marc-Andre-Rivet deleted the exp-dynamic branch November 19, 2019 19:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants