-
Notifications
You must be signed in to change notification settings - Fork 30
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
Improve the Plots
UI (regular and comparison)
#1561
Comments
This comment was marked as outdated.
This comment was marked as outdated.
Concept preview: Keypoints:
@mattseddon @sroy3, please take a look and give me your feedback 🙌 |
Plots
UI (regular and comparison)
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. Plots webview having its own state/ability to select-deselect revisions
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: Screen.Recording.2022-05-02.at.10.50.40.am.mov☝🏻 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 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. Chart tooltipsHappy to update the Feedback given by @sroy3
For template plots we can filter the passed data using the 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. On resizing of plots:
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: Screen.Recording.2022-05-02.at.11.00.16.am.mov |
Can we 'pretend' to toggle the graphs?
See your point. Well, let's use no closing |
I see two approaches here:
I would not sync the experiments tree with the
Yes, filters must work for both |
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. |
|
This comment was marked as off-topic.
This comment was marked as off-topic.
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. |
WDYT about this concept:
Like it! 👌
Talked to @shcheklein about the concept of toggle visibility + toggle revision. Agreed to have just toggle revision 🙌 🏁 So the scope now:
|
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). Screen.Recording.2022-05-05.at.1.01.26.pm.movLMK what you think Edit: opened #1659 to discuss |
Nicely done! Just wondering if we can avoid using the I'd expect the following scenarios:
|
@yalozhkin I completely forgot that we can switch the icon dependent on a context value within VS Code. What do you think of this: Screen.Recording.2022-05-06.at.12.21.45.pm.movOn filters being applied immediately - we do this for the experiments table. I would hope the real workflow would look something like this: Screen.Recording.2022-05-06.at.12.21.45.pm.movThe 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. |
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 #1561 (comment) I actually experimented on how the single container elements can be spaced including the |
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. |
@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 The inside of a jpg looks like this
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. |
@sroy3 @mattseddon updated the task body with Figma specs 🔝 |
@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. @shcheklein please let me know what you have in mind. Thanks! |
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).
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 |
My comments in Figma Thank you ! @mattseddon @shcheklein @yalozhkin |
@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 #1254? |
Scope:
Proposed design
Preview:
Figma Plots UI:
The text was updated successfully, but these errors were encountered: