Closed yalozhkin closed 2 years ago
@sroy3 @mattseddon could you please take a look at this refined concept for Plots
:
Keypoint: selected experiments are visible and always on top (users can toggle their visibility and hide particular experiments, also they can launch the selection form using the N of 7
button)
The Plots
toolbar states:
A particular experiment interaction (from left to right, off/on variants):
Default → Show the Hide
button on hover → Show the Hide
hint precisely on the Hide
iconic button
A particular plot interaction:
Hide
buttonA particular experiment interaction (from left to right, off/on variants):
Default → Show the
Hide
button on hover → Show theHide
hint precisely on theHide
iconic button
Tested the fulfilled buttons with different colors, looks not that good for bright colors:
SO another option is to fill the boxes only:
Suggestions for improving consistency between regular plots and comparison table plots:
Concept preview:
Keypoints:
Plots
(so we don't need toggling plots to select/unselect in the experiments panel anymore)@mattseddon @sroy3, please take a look and give me your feedback 🙌
I think the concept looks nice. Here are the concerns/questions I have:
@mattseddon correct me if I'm wrong on any of these points.
Things I can easily start working on:
Thank you @sroy3 for your feedback!
Could you please also check these two concepts for resizing the plots? I found the small-regular-large approach confusing, so there are some ideas:
@yalozhkin the designs/concepts look really good. Comments/questions are below.
Keypoint: selected experiments are visible and always on top (users can toggle their visibility and hide particular experiments, also they can launch the selection form using the
N of 7
button)
- Toolbar for
Plots
(so we don't need toggling plots to select/unselect in the experiments panel anymore)
I like the concept of giving the webview its own state/way to select revisions but I have a few questions:
How is the initial list of revisions generated (i.e the list of up to 7 revisions)?
Can experiments/revisions still be selected/deselected from the experiments webview and/or experiments tree?
Will we still provide the user with the ability to auto-select the revisions to the current filters? I.e this:
☝🏻 this is something that Alex/David asked for here
How do we make the fact that "selected experiments are the superset of revisions to select from" an easy concept to understand for users? Could we change the name of the EXPERIMENTS
tree to EXPERIMENTS/REVISIONS
and add both show plots
and show experiments
icons to the menu title to show that both are affected by this tree?
I think that adding the revisions to the webview gets us partway there but this concept not being intuitive is the most consistent piece of feedback that we are getting. GTM feedback here.
Happy to update the Experiments Checkpoints
tooltips as we are in control of those. However, the tooltips shown in the Plots
section are provided by the template they are generated from. We just display whatever the user has set in their template. Updating these on a case by case basis is not maintainable.
- The revisions list at the top currently has 2 actions on it. Toggle the visibility and remove the revision (unselect it from the tree). This could work for the comparison table, but the other plots a revision consists of a line on the graph and isn't something that can be toggled directly in the webview. In summary the toggling would only fit the comparison table, while to "x" action could work for all.
For template plots we can filter the passed data using the rev
field (content.data.values
) but this will only work for vega plots, we will have to add more parsing/logic whenever more plot types are added 😬 . We would also need to add a check for the values array being non-empty so that we display only plots when there is data otherwise show an empty state.
Question: If I remove a revision from the comparison table does that deselect the same revision from the whole webview, deselect the revision completely or just remove it from the comparison table?
Everything else looks correct there.
Could you please also check these two concepts for resizing the plots? I found the small-regular-large approach confusing, so there are some ideas:
We do have the "zoomed plot" concept now. Please take a look at this and give us some feedback. The reason for adding this is that the overall view should serve as a customisable dashboard. When the user see something that they want to focus on this should be easy without having to resize all of the plots.
Demo:
@sroy3
This could work for the comparison table, but the other plots a revision consists of a line on the graph and isn't something that can be toggled directly in the webview. In summary the toggling would only fit the comparison table, while to "x" action could work for all.
Can we 'pretend' to toggle the graphs?
Once the user toggles an experiment off we consider this similar to the hiding x
, replace plots but do not remove the experiment revision from the webview and show it turned off instead?
Currently, there are no way to filter on these, but that could be done, but why would we need to remove it if we can collapse a row and re-order them with drag and drop (I'll implement this as it is not there at the moment)? I'm asking because the way to re-add the row would be to do it from the side panel (and we'd need to add yet another tree for it).
See your point. Well, let's use no closing x
button for the comparison plots.
@mattseddon
How is the initial list of revisions generated (i.e the list of up to 7 revisions)?
I see two approaches here:
Can experiments/revisions still be selected/deselected from the experiments webview and/or experiments tree?
I would not sync the experiments tree with the Plots
and Experiments
webviews.
Will we still provide the user with the ability to auto-select the revisions to the current filters? I.e this:
Yes, filters must work for both Plots
and Experiments
webviews
@sroy3
This could work for the comparison table, but the other plots a revision consists of a line on the graph and isn't something that can be toggled directly in the webview. In summary the toggling would only fit the comparison table, while to "x" action could work for all.
Can we 'pretend' to toggle the graphs? Once the user toggles an experiment off we consider this similar to the hiding
x
, replace plots but do not remove the experiment revision from the webview and show it turned off instead?
I think this could be possible. It might be a little harder keeping track of the maximum number of revisions though. If there are 7 and I simply toggle one off, the extension now sees I have 6 revisions and would allow me to select a new one, making it impossible then to re-toggle the revision.
Concept preview:
Keypoints:
- Toolbar for
Plots
(so we don't need toggling plots to select/unselect in the experiments panel anymore)- Uniform container (color, corners, spacing) and actions (hover menu, drag, and hide) for all types of plots, including comparison rows
@mattseddon @sroy3, please take a look and give me your feedback 🙌
Sharing some mid fidelity explorations, using a 12px base. The detailed list of comments will be developed at a more advanced stage.
This is something I noticed during the meeting: When zooming in a single plot the colors are not consistent with the current theme.
This is something I noticed during the meeting: When zooming in a single plot the colors are not consistent with the current theme.
They are, it's simply another theme color. The plots have a transparent background so we cannot use the same. We can use the normal background color + the transparent plot background to get the same color. I'll do this one now, it should be quick.
@mattseddon
How do we make the fact that "selected experiments are the superset of revisions to select from" an easy concept to understand for users? Could we change the name of the EXPERIMENTS tree to EXPERIMENTS/REVISIONS and add both show plots and show experiments icons to the menu title to show that both are affected by this tree?
WDYT about this concept:
view/title
of Columns
(instead if Metrics & Params
show the Lab icon → Show tableview/title
of Plots
show the Plots icon → Show plotsWe do have the "zoomed plot" concept now. Please take a look at this and give us some feedback. The reason for adding this is that the overall view should serve as a customisable dashboard. When the user see something that they want to focus on this should be easy without having to resize all of the plots.
Like it! 👌
@sroy3
I think this could be possible. It might be a little harder keeping track of the maximum number of revisions though. If there are 7 and I simply toggle one off, the extension now sees I have 6 revisions and would allow me to select a new one, making it impossible then to re-toggle the revision.
Talked to @shcheklein about the concept of toggle visibility + toggle revision. Agreed to have just toggle revision 🙌
🏁 So the scope now:
Plots
webviewview/title
leading to the Plots
@yalozhkin
WDYT about this concept:
- On the
view/title
ofColumns
(instead ifMetrics & Params
show the Lab icon → Show table- On the
view/title
ofPlots
show the Plots icon → Show plots
updated in #1655 + #1656
Please watch this video as it demonstrates the concept of linking the Experiments tree to both the table and plots webviews by changing the view's title and using of new view title icons/actions (first mentioned here).
LMK what you think
Edit: opened #1659 to discuss
@mattseddon
Please watch this video as it demonstrates the concept of linking the Experiments tree to both the table and plots webviews by changing the view's title and using of new view title icons/actions (first mentioned https://github.com/iterative/vscode-dvc/issues/1561#issuecomment-1114419179).
Nicely done! Just wondering if we can avoid using the Filter
(apply filter?) icon here.
I'd expect the following scenarios:
Plots
/Experiments
icons opens the webviews → See unfiltered plots/experimentsacc > 0.8
→ See filtered plots/experiments (immediately)@yalozhkin I completely forgot that we can switch the icon dependent on a context value within VS Code. What do you think of this:
On filters being applied immediately - we do this for the experiments table. I would hope the real workflow would look something like this:
The demo shows why we don't always automatically apply filters to select experiments. If there are ever more than 7 experiments remaining we have to send a user a message to ask them to select the most recent (not even sure this is correct) or cancel the action. This could quite easily lead to either unexpected or unwanted behaviour and bombarding the user with messages. Hope this makes sense, happy to expand further if necessary.
A few comments on the latest extension version (sorry, if that was mentioned before):
Here is another example:
All there groups have so different sizes and all of them are not comfortable by default I would say.
1.Just a heads up that some of the current section that uses plots in image format (Static plots) probably may be updated with the vector version. Which will help to have better consistency in the Plots view in general. @sroy3 can you verify this please?
2.Here https://github.com/iterative/vscode-dvc/issues/1561#issuecomment-1116410596 I actually experimented on how the single container elements can be spaced including the spacing rules
.
1.Just a heads up that some of the current section that uses plots in image format (Static plots) probably may be updated with the vector version. Which will help to have better consistency in the Plots view in general. @sroy3 can you verify this please?
Static plots are rendered as svgs. The images are in the comparison table and are being sent as such.
Static plots are rendered as svgs. The images are in the comparison table and are being sent as such.
I thought that we could use exactly the same design for any plots - static or dynamic, since probably with some magic we could use the XML and render it exactly as we do for ‘html’ plots. Does it make sense @sroy3 ?
Static plots are rendered as svgs. The images are in the comparison table and are being sent as such.
I thought that we could use exactly the same design for any plots - static or dynamic, since probably with some magic we could use the XML and render it exactly as we do for ‘html’ plots. Does it make sense @sroy3 ?
@maxagin When it comes to images (anything that currently goes into the comparison table) we are provided with the path to static images by DVC, e.g /path/to/image/on/disk/misclassified.jpg
.
Hopefully, this explains why we generally cannot change the styles inside of an image once it has been created.
We also cannot easily update plots based on templates to have rounded lines and we could very easily be overriding the interpolation method that the user has specifically set in their template. IMO it would be better to leave line plots as is.
@maxagin @yalozhkin folks, instead of consolidating this, we keep exploding number of tools, tickets, etc. It's quite hard to get sense of what is where, what is expected, etc. Would be helpful to reduce the complexity somehow 🙏
The multi points review or feedback better be located somewhere else, but not inside the main GitHub ticket, simply because the feedback needs to be worked out separately. And if there (in feedback) something that makes sense, after discussions it can be moved into the main ticket.
It is good if we can establish a set of tools we can use. We could see that GitHub (linear comment approach) is not a good solution for this feedback. The Figma also was not planned to be used as I wanted to include screenshots.
It is why I have shared the google docs. Maybe we could use Notion for this type of “articles”. What do you think?
@shcheklein please let me know what you have in mind. Thanks!
The multi points review or feedback better be located somewhere else, but not inside the main GitHub ticket,
yep, in this case it'a more like even design process, design UX discussion. Agreed that GH is not the best for this. I would move to GH updates / final stages, or product spec discussions (like we did for the experiments labeling story).
The Figma also was not planned to be used as I wanted to include screenshots
But those screenshots come from Figma primarily in this case? But even if not, I this we still could include them in Figma. And it supports non linear feedback. I would try to keep it there first. Especially when we are discussing some design proposal. In this case we could have avoided hopefully and additional doc and and an additional ticket, wdyt?
Sure @shcheklein . Will be moving all the comments to Figma
@sroy3 did you have time to check the specs? Please let me know if there are any questions.
@sroy3 did you have time to check the specs? Please let me know if there are any questions.
Everything looks good to me. I'm starting working on changing things today. We don't have static images elsewhere but in the comparison table, so that's one less thing to worry about.
Hey, folks, I know all these discussions were a bit chaotic (we are working on improving it!) and it was easy to miss the review request. However, have you had a chance to review my questions, comments and suggestions in Figma regarding the Improve the Plots UI #1561 ticket, which includes things like:
and more. Especially major thing like:
and more. That I believe might improve certain things . Let me know when / if it’s done, would be nice before we start the implementation.
Should include https://github.com/iterative/vscode-dvc/issues/1254?
Scope:
Proposed design
Preview:
Figma Plots UI: