-
Notifications
You must be signed in to change notification settings - Fork 1
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
Migrate to react-charts #28
Merged
Merged
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
react-vis hasn't been maintained for a long time and react-charts seems like the most promising react charts library ...
... to match the style of the old chart
Null values were still showing, sometimes on top of other series points and sometimes underneath. When showing underneath another point there was a small outline artifact
By default, each series point/datum circle 'moves' into place
This is nicer anyway since it moves all of the related x-axis code together inside the primaryAxis function
... and rename SeriesPointWithHint to something shorter but still just as descriptive :)
- Add right Y axis - Remove redundancy in generateLeftYAxisLabels/generateRightYAxisLabels - Update generateLeftYAxisLabels/generateRightYAxisLabels to filter out zero values - Fix sort for generateRightYAxisLabels
- When a series is focused, make the focused series' line and circles slightly thicker - When a series is focused, make the other series' line and circles more transparent - When no series is focused, return all series to normal formatting
The default is to show all data for the selected x-axis value
Unfortunately it still shows zero values and the order seems to be set based on the first value
Also rename hintTitle and hintValue for more clarity and clean up some more react-vis code
This fixes the following React warning: > Warning: Cannot update a component (`Chart`) while rendering a different component (`TooltipRenderer`).
This fixes the following React warning: > Warning: Cannot update a component (`Chart`) while rendering a different component (`Line`)
- Add more spacing to top of button group since initially it's too small for the bottom button group (although annoyingly it changes when the window's resized; this still needs to be dealt with) - Limit the chart width to 800px and make sure it's always centered using flex - Move button-group-grid CSS to ButtonGroup.css - Remove remaining react-vis code - Uninstall react-vis
This also lets us remove the explicit min/max 😁
[skip ci]
bmaupin
added a commit
that referenced
this pull request
Mar 15, 2023
Migrate to react-charts Closes #26
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.
This offers a number of advantages:
The downside is that it has most features enabled out-of-the box so they need to be opted out (react-vis was opt-in) which ends up being more code in the end 🤷♂️