-
Notifications
You must be signed in to change notification settings - Fork 0
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
mrc-5455-v2 POC: Can add graphs to Run and drag variables to select or hide #205
base: main
Are you sure you want to change the base?
Conversation
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.
Works nicely. I like the gradient colors of the variable buttons. Could extend out the range to include purple at one end, in case someone has a whole lot of variables.
Also, possibly useful to be able to select and drag more than one variable at once? (shift+click to select more than one.) But might not add enough value to be worth bothering to implement.
<div class="ms-2"> | ||
Drag variables to move to another graph, or to hide variable. Press the Ctrl key on drag to make a | ||
copy of a variable. | ||
</div> | ||
<selected-variables v-for="graphKey in graphKeys" :graph-key="graphKey"></selected-variables> | ||
<hidden-variables style="clear: both"></hidden-variables> | ||
<button class="btn btn-primary mt-2 float-end" id="add-graph-btn" @click="addGraph">Add Graph</button> |
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.
Should we only show this if there are multiple graphs? Otherwise, it's a bit confusing because you can't in fact drag the variables, yet.
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.
Oh, I see, you can drag them already, to the hidden variables section.
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 guess I overlooked the 'hidden variables' bit at first while I was trying to suss out what the interaction was by trying to drag. So I wonder how this could have been more obvious to me on first seeing the feature. I should have created a graph first rather than trying to drag ineffectually.
Here are some ideas...
One half-baked idea is that there could be a 'graph N+1' (as it were) that you can always drag to, such that when you've only got Graph 1, you can still drag variables onto a new graph (so, in this case, Graph 2), and dragging variables into the next graph causes the graph to start existing? Would perhaps need to grey out this draph graft.
A better iteration on that idea: when you start dragging a variable, if there is only one graph in existence, then a field appears to receive them, 'Drag variables here to create a new graph with these variables'.
Or, edit the instructions to mention the possibility of adding a graph or change the implied order of trying out the functionality, e.g.:
Drag variables to 'Hidden variables' to remove them from your graph, or click 'Add Graph' to create a new graph to move them to. Press the Ctrl key on drag to make a copy of a variable.
Or, move the 'add graph' button to sit before the 'hidden variables' bit, because the 'add graph' button is well into my peripheral vision when I'm reading the instructions, I wouldn't spot it unless looking for it. If moving it to this position, I'd make it into a +
icon for concision.
Great work honestly looks really snazzy and works like a charm!!!!! Didnt review code itself as only a POC... just some minor suggestions/questions with UI/UX
|
Yeah, nice idea. As I recall, the colour scales was stolen from the old Shiny version. I think B -> R works nicely for a small number of colours as it keeps the ends of the scale very different, but we could add in the purples if you have lots of variables.
That would be nice too - I imagine most of the time users will want to see the effect of dragging one variable, but could also be useful to do multiple if it's a well known model. Will add tickets for both of these. |
Yep, Rich made this suggestion already, and there is a ticket for it. We'd have that even if there are already multiple graphs. We'll still keep the Add button as an alternative.
Nice, will do.
Yeah, it's definitely not quite right as it is. So maybe it does slot in above Hidden Variables, and the button transforms into the "Add to new graph" drop area (when implemented) on drag. |
Yeah, and as David noted, users won't necessarily notice the Hidden variables area. One option would be for all variables to have the 'x' button, not just duplicates, and clicking that will move to Hidden (if there are no other instances).
Agreed - I just haven't thought of a better title yet. Maybe just "Graphs", but I quite like that "Select variables" gives you a clue about what the idea is behind having these graphs.
As I understand it, researchers are likely to be mostly wanting to shuffle round a single set of variables, assigning some to be hidden all the time, with occasional need to duplicate a variable in a couple of graphs. So this seemed to be a friendlier UI than making them use the plotly trace toggle. |
This proof of concept demonstrates an interface allowing the user to add more graphs, in order to show different sets of variables (for which different scales may be appropriate) on each graph. You can run it up locally or it is currently deployed to wodin-dev: https://wodin-dev.dide.ic.ac.uk/preview/apps/defaultcode/
From the Selected Variables area on the Code Tab, the user can now "Add graph" from a button and drag variables between graphs, or to "Hidden variables" to remove from all graphs. This currently just adds graphs to the Run Tab, but we'll also include new graphs on Sensitivity too, and will ensure that all graphs' Time axes are identical.
You can press Ctrl key when drag to make a duplicate variable in another graph. Duplicates have an 'x' button to remove that instance only (or you can also drag to Hidden from one graph, which will still leave it behind on any others).
When we implement for real, we'll tidy up the code including:
graphs
configuration an array rather than a dictionary, as the keys aren't significant, and we don't let users edit the graph names anyway.model
I'll implement what's here "for real" in a couple of separate branches with tests etc, so this PR shouldn't be merged. Would be great to get your input on the usability in particular.
Still TODO (not in POC):
When you test, you might want to use the Ebola model code to give you more variables to play with: