-
Notifications
You must be signed in to change notification settings - Fork 4
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
Refactor plots to use plotly.js instead of chart.js (part of DSEGOG-57) #564
Draft
louise-davies
wants to merge
12
commits into
develop
Choose a base branch
from
refactor-plots
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,760
−2,109
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- WIP, still need to migrate traces
- also remove some chart.js references
Also update CDN url for plotly to more optimised version
- fix some issues with the plotting code & styling - also use better screenshot command & make webkit snapshots not as large
- mostly just accounting for the webkit change & switching to better screenshot method
- Had to change some layout stuff to make things resize properly/show the right size in tests
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #564 +/- ##
===========================================
- Coverage 97.86% 97.85% -0.01%
===========================================
Files 94 94
Lines 12531 12496 -35
Branches 2025 2053 +28
===========================================
- Hits 12263 12228 -35
+ Misses 266 264 -2
- Partials 2 4 +2 ☔ View full report in Codecov by Sentry. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Motivation for this change is DSEGOG-57 - Plotly has the
rangebreaks
feature which can implement the skip non-working hours feature. That change will be in a follow up PR to try and keep this PR smaller as it's already a behemoth!Overall the plots should work identically to before - the only difference I can note is that we can't easily properly limit the zoom/pan range to the range of the data anymore. But since that only affects zoom/panning I decided it's not that big of a deal as the user can just choose to keep within bounds or not.
I had to rearrange some of the styling in the windows to accomodate Plotly to ensure the plot is correctly responsive and fills up the container as it is supposed to.
Also, some minor improvements in the playwright tests - we now use a better screenshot method (it takes two screenshots and if they're not the same it will continue taking subsequent screenshots until the page is stable before comparing). Also changed it so that Webkit screenshots are no longer huge - this is due to the default DPI of Webkit being 2 instead of 1.
Testing instructions
Agile board tracking
connected to DSEGOG-57