Switch experiment table radio buttons to plot icons #4121
Conversation
|
Thanks @mattseddon ! It gets us aligned with Studio. Can we make them close to the checkbox? (same way as in Studio). |
Do you want me to recreate this: 
? Where do stars go? Our checkboxes currently behave very differently. |
|
We also specifically put a wider gap between the checkboxes and the actions for #3430 to help separate them from one another (different behaviour) |
No, I'm not sure we can do that (in our case top level can also be in the running state). We can replace the plot icon with a spinner when it runs. or put a spinner on top of it, I don't know tbh ...
after plots?
in Studio its WIP. They will behave similar. Do you mean some specific things?
It thought this because we have those spinners? |
| [styles.oddRow]: isOdd, | ||
| [styles.evenRow]: !isOdd, | ||
| [styles.workspaceRow]: isWorkspace, | ||
| [styles.normalRow]: !isWorkspace, |
There was a problem hiding this comment.
[F] Half of these classes didn't exist, the other half have been deleted.
| @@ -576,7 +580,7 @@ $badge-size: 0.85rem; | |||
| height: 100%; | |||
|
|
|||
| &:first-child { | |||
| margin-right: 20px; | |||
| margin-right: 4px; | |||
There was a problem hiding this comment.
[Q] Will changing this affect the way the shadow is displayed? Is there a gotcha here? Maybe with the default size of the first column?
There was a problem hiding this comment.
Changing margin shouldn't affect the shadow or the width of the first column. It just affects the width of the exp name text. By decreasing the margin, the exp-name/commit has a bit more room :)
mattseddon
force-pushed
the
switch-plot-icons
branch
from
7b123e0 to
0228dd9
Compare
mattseddon
force-pushed
the
switch-plot-icons
branch
from
0228dd9 to
1f6b725
Compare
| border: calc(var(--border-width) * 1px) solid $checkbox-border; | ||
| color: $icon-color; | ||
| .plotBox { | ||
| height: $checkbox-size; |
There was a problem hiding this comment.
There is little to no spacing between the SVG and the border, which looks strange to me. I do see the value of making the plot icon look more clickable though and the checkbox look does help with that.
We could add a little padding to the button, but that might make it harder to tell if the exp is selected since the SVG is smaller:
There was a problem hiding this comment.
Have we considered filling in the checkbox completely with the selected color? That would make selected experiments stand out more to me. Example:
There was a problem hiding this comment.
And I now see that I'm basically just describing what Studio does in their design 😅
There was a problem hiding this comment.
What color did you use for the selected svg fill in that screenshot?
There was a problem hiding this comment.
Updated to this:
WDYT?
There was a problem hiding this comment.
I just used white since it works as a foreground for all of our unchanging custom colors. Something likevar(--vscode-list-activeSelectionForeground) works for most cases if we wanted to use a variable though!
There was a problem hiding this comment.
Updated to this:
WDYT?
Looks better to me!
There was a problem hiding this comment.
I went with fill: 'var(--vscode-editor-foreground)' for the selected svgs we can revisit later if needed.
mattseddon
force-pushed
the
switch-plot-icons
branch
from
d20eea6 to
9399425
Compare
mattseddon
force-pushed
the
switch-plot-icons
branch
from
9399425 to
657829b
Compare
|
Code Climate has analyzed commit 657829b and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (85% is the threshold). This pull request will bring the total coverage in the repository to 95.2% (0.0% change). View more on Code Climate. |
Related to #3430
This PR swaps the old radio button for a plot icon and separates an experiment's running indicator from the icon. This brings us closer to the new Studio UI for plotting.
Demo
Final Icons
Layout (outdated icons)
Screen.Recording.2023-06-15.at.7.29.05.pm.mov
Studio UI (for reference)
Note: As you can see in the original demo (below) I have left the tree untouched. I have taken this approach because the user will lose information if we move to the plot icon there (a user cannot tell if an experiment is running from the tree).
Demo (outdated)
Screen.Recording.2023-06-15.at.11.11.45.am.mov