Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add interactive plots with Plotly #163
base: main
Are you sure you want to change the base?
Add interactive plots with Plotly #163
Changes from 13 commits
0a50ca2
23bdaaa
73d0ee4
406710d
ba7edd8
954a603
b4985b5
c971876
74c3ce2
2e7aea5
d62d08a
59b37bc
a898191
02df95b
e4bd48d
d271eee
226111e
24a9aa2
121578f
152d94a
3c32907
89a5c3d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the card HTML can contain a REST endpoint, and fetch the data with a second call? That way we don't have to generate and include all the raw data in the HTML page. This should help a lot with page responsiveness. You can see examples of REST calls in other modules:
If you need help with this, let me know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have implemented this in 226111e. The page loading is way more responsive now.
Still, I'm a total newbie with Flask, so a thorough review is needed.
My main concerns are:
PlotlyView
module. I decided to hash theid(self)
which should work, but has the disadvantage that each time the dashboard is launched the URLs change. I don't think this is a huge problem, but I feel like there is a better way to do this.register
method is called. However, the assets URLs should only be added once. If you try to do it multiple times, the second time around the module is not registered. I have solved this with a class attributePlotlyViewer._assets_url_registered
that avoids this duplication. Still, I feel like this solution is somewhat hacky.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@javierbg
Flask Blueprints are the way to go. I started experimenting with them in #182 to allow multiple Notes modules to be added.