mrc-5490 Add graph #207
Merged
mrc-5490 Add graph #207
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
absternator
reviewed
Jul 2, 2024
absternator
reviewed
Jul 2, 2024
absternator
reviewed
Jul 2, 2024
| const unselectedVariables = | ||
| rootState.model.odinModelResponse?.metadata?.variables.filter( | ||
| (s) => !payload.selectedVariables.includes(s) | ||
| ) || []; | ||
| commit(GraphsMutation.SetSelectedVariables, { ...payload, unselectedVariables }); | ||
| // sort the selected variables to match the order in the model | ||
| const selectedVariables = payload.selectedVariables.sort((a, b) => |
There was a problem hiding this comment.
is there a benfit of the sort? is there significance in order of variables
There was a problem hiding this comment.
Yeah, this is the variable order that will be displayed in the UI, and we want it to match the order of variables as they're defined in the model, which might typically follow a temporal order e.g. S(usceptible), I(nfected), R(ecovered) or be ordered by category. Just leaving them unordered or sorting alphabetically would be less meanginful for the user. This matches the previous toggle behaviour where variables were always in model order.
absternator
reviewed
Jul 2, 2024
absternator
reviewed
Jul 2, 2024
mrc-5551 Show Time label on final graph only
…te when graph count changes
mrc-5485 Use multiple graph config in Sensitivity plots
Co-authored-by: David Mears <[email protected]>
mrc-5545 Do not show linked variables on graphs which are not showing those variables
mrc-5443 Copy variable on drag if Ctrl key is pressed
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.
NB After approval, we'll use this as the 'epic branch' for multiple graph tickets, and merge to main when the feature is complete.
This branch includes functionality to add graphs, and drag variables between graphs or to Hidden variables, as in the proof of concept.
Sorry about the line count! Future branches will be smaller.
Key changes:
GraphConfigsto replaceSelectedVariablesand act as a container for the individualGraphConfigs and theHiddenVariablescomponent.GraphConfigsCollapsiblewhich has the top level vertical collapse and shows nothing if no variables or compile required - this is easier to unit test by splitting like this because verifcal collapse content is contained in a slot.GraphConfigandHiddenVariablesis contained in aSelectedVariablesmixin.colorpackage without enabling synthetic default imports - which caused a whole raft of other compilation errors, so I just side-stepped that by wrapping the usage of color (to fade the colour of hidden variables) into a separate util, and mocking that..