Skip to content
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

Experiments table: improve styles #2070

Closed
maxagin opened this issue Jul 20, 2022 · 32 comments
Closed

Experiments table: improve styles #2070

maxagin opened this issue Jul 20, 2022 · 32 comments
Assignees
Labels
A: experiments Area: experiments table webview and everything related priority-p2 Future feature, less priority for now

Comments

@maxagin
Copy link
Contributor

maxagin commented Jul 20, 2022

EXPs table improvements. Original ticket https://github.com/iterative/design/issues/28

Figma

Rest state

  1. Add a bottom line to every row and remove even rows BG style
  2. Keep the header BG colour the same as the table body BG colour
  3. Use the same line style for header item separators
  4. Keep the caret the same colour as the other icons on the left side

Hover state

  1. Show hover styles (column, row) when the cell is on hover
  2. "lr" on hover. Also, show the hover style for the column
  3. "global" on hover. Also show the hover style for the "seed", "lr", "weight_decay" and three columns
  4. The first column cell on hover. Checkbox and Star rest styles (same like without hover)

Resize columns

  1. When resizing the cell sideline grows from 1px to 2xp and changes colour
  2. When resizing the hover styles are applied the same way as if the block is on hover.
  3. The resize is always applied to the right side of the column
  4. The resize is activated only in the header of the table when hovering on the sidelines

Select and star EXP/s

  1. When a selected row is on hover, add a shadow to show the hovered row
  2. New (lighter) color for the selected row background. One of the reasons: we do not want to have it similar to the color that is used for the buttons
  3. Selected rows have different bottom borders and left/right for selected cell color

Star and checkbox behaviour

  1. Star
  2. Checkbox
@maxagin maxagin added the A: experiments Area: experiments table webview and everything related label Jul 20, 2022
@maxagin maxagin self-assigned this Jul 20, 2022
@shcheklein shcheklein added the priority-p2 Future feature, less priority for now label Jul 20, 2022
@maxagin
Copy link
Contributor Author

maxagin commented Jul 21, 2022

Note: In a composition with these changes, we may want to rethink the sticky panel shadow and implement one from the design. I think the best if we could see both examples. Screen recording (video) probably will do the job. cc @julieg18

@julieg18
Copy link
Contributor

Note: In a composition with these changes, we may want to rethink the sticky panel shadow and implement one from the design. I think the best if we could see both examples. Screen recording (video) probably will do the job. cc @julieg18

When you say "stick panel shadow", are you referring to the header shadow? Currently, the experiments column shadow should match the design on Figma while the header shadow is a little different from the Figma design :)

@maxagin
Copy link
Contributor Author

maxagin commented Jul 22, 2022

When you say "stick panel shadow", are you referring to the header shadow?

Yes @julieg18 . Thanks

Currently, the experiments column shadow should match the design on Figma while the header shadow is a little different from the Figma design :)

Cool. I just wanted to say that it's going to be easier to decide about the shadow after other changes are implemented. However, if you want, you can implement it right now like in Figma :)

Q: How actually does the shadow change from theme to theme? Just for example: box-shadow: 3px 3px rgb(51 51 51 / 56%); is just RGB value is different? Thanks @julieg18

@shcheklein
Copy link
Member

Just to make sure that we are on the same page - we should be using the same shadows as VS Code already has (e.g. command palette).

@maxagin
Copy link
Contributor Author

maxagin commented Jul 22, 2022

we should be using the same shadows as VS Code already has

@shcheklein there is a variety of options. Here are just a few examples used in VSCode (Visual Studio Code Toolkit):

Screen Shot 2022-07-21 at 11 05 21 PM

Screen Shot 2022-07-21 at 11 08 24 PM

Screen Shot 2022-07-21 at 11 24 59 PM

Also, I would not be so strict, as the rules change all the time. If we want to be a real conservative we will have to change the table, rows and other interactive elements.

We do not follow this:

Screen Shot 2022-07-21 at 11 48 09 PM

Screen Shot 2022-07-21 at 11 51 57 PM

Apropos rounded corners. I think we can come down a bit and try to be reasonable. I must to say my obsession with the straight corners for VSCode was just an obsession :) I am trying to fix this

Screen Shot 2022-07-21 at 11 52 48 PM

Screen Shot 2022-07-22 at 12 00 25 AM

Screen Shot 2022-07-22 at 12 00 41 AM

@maxagin
Copy link
Contributor Author

maxagin commented Jul 22, 2022

Bottom line I think we should use what exists as much as possible, but if we clearly have to use custom elements we should, for example, the errors, in this case, the usage of the X is wrong IMO. So this is a good example where we better use elements that are familiar to our users because we use them in other places (!).

@julieg18
Copy link
Contributor

Cool. I just wanted to say that it's going to be easier to decide about the shadow after other changes are implemented. However, if you want, you can implement it right now like in Figma :)

I don't think I have access to editing in Figma, but here is the box-shadow used for the header 0 5px 8px -2px var(--vscode-widget-shadow); (X: 0, Y:5px, B: 8px, S: -2px).

Q: How actually does the shadow change from theme to theme? Just for example: box-shadow: 3px 3px rgb(51 51 51 / 56%); is just RGB value is different? Thanks @julieg18

Yes! The other values stay the same with the RGB value changing depending on the theme.

@maxagin
Copy link
Contributor Author

maxagin commented Jul 22, 2022

The changes in this ticket will dictate what shadow is going to be used. Let's keep for now the shadows you already have and make sure to discuss them again when new styles are implemented. Thank you @julieg18 ! Can not wait to see the progress on this one :)

@shcheklein
Copy link
Member

@maxagin thanks, it has a lot of stuff that is not related (shadows, plots), but still doesn't answer basic questions - color schema (how do we pick color for borders?)

Let's simplify, restructure and think through the implementation a bit.

Things that come to my mind right away:

  • the whole column selection on hover can make everything too noisy
  • multiple columns selection can make it even more noisy
  • color for when row is selected (checkbox) is not specified?

@maxagin
Copy link
Contributor Author

maxagin commented Aug 7, 2022

but still doesn't answer basic questions - color schema (how do we pick color for borders?

We have just figured out that the #000000 was deprecated, but it does not mean that the rest of the toolkit is outdated - this would be a small catastrophe :) The new spec in Figma answers all the questions about the colours (I hope).

the whole column selection on hover can make everything too noisy

Do we want to try my version, would you like to discuss other options, or should I remove this?

color for when row is selected (checkbox) is not specified?

It is mentioned in the spec that there are styles for Checkbox and Star in the row hover state.

@shcheklein
Copy link
Member

It is mentioned in the spec that there are styles for Checkbox and Star in the row hover state.

What are those styles exactly? I don't see where it's specified

Do we want to try my version, would you like to discuss other options, or should I remove this?

we can try it if you feel it's reasonable

The new spec in Figma answers all the questions about the colours (I hope).

I don't see the answers there tbh. Primarily about border colors for examples (that's where we were trying to apply #0000?)

@maxagin
Copy link
Contributor Author

maxagin commented Aug 8, 2022

What are those styles exactly? I don't see where it's specified

Here is an example of how to use the Inspect mode. In the spec you just point out what needs to be done, then the dev will use the Inspect mode to get all the values.

Screen.Recording.2022-08-08.at.12.26.02.PM.mov

I don't see the answers there tbh. Primarily about border colors for examples (that's where we were trying to apply #0000?)

This is the same as above. Please let me know if you would like to have more clarification. The color for the border in spec is 1e1e1e.
Screen Shot 2022-08-08 at 12 32 38 PM

The darkest color I can find in the default theme is Base 21 or 1e1e1e - Julie. Friday Slack message

@maxagin
Copy link
Contributor Author

maxagin commented Aug 8, 2022

@julieg18 hi, I have updated the TODO list with the latest fixes in the ticket description. The Figma spec was redesigned. Let's meet today a bit later as we planned to go thru everything.

@shcheklein
Copy link
Member

Please let me know if you would like to have more clarification. The color for the border in spec is 1e1e1e.

Is it the same in the white theme? Green theme?

Base 21 or 1e1e1e

curious - What is Base 21?

@maxagin
Copy link
Contributor Author

maxagin commented Aug 8, 2022

Please let me know if you would like to have more clarification. The color for the border in spec is 1e1e1e.

Is it the same in the white theme? Green theme?

The Base 21 or 1e1e1e in our case is a variable. For the default dark theme, it is a tint of black. In other themes, it will look different, but it will be a tint of the related base color. For example:

  • VSCode dark theme: Border is dark, BG is a bit lighter dark
  • Green theme: Border is green, BG is a bit lighter green

@shcheklein
Copy link
Member

@maxagin where this variable is being used in the VS Code itself? (if it's used at all). Also, is there a way from Figma spec to understand that we should be taking Base 21 var value?

@julieg18
Copy link
Contributor

julieg18 commented Aug 8, 2022

@maxagin where this variable is being used in the VS Code itself?

Looks like 1e1e1e is being used in 9 variables, all have something to do with background:
--vscode-breadcrumb-background
--vscode-editor-background
--vscode-editorGutter-background
--vscode-editorMarkerNavigation-background
--vscode-tab-activeBackground
--vscode-tab-unfocusedActiveBackground
--panel-view-background
--vscode-editorPane-background
--vscode-panel-background

@shcheklein
Copy link
Member

so, 1e1e1e is not a variable itself, right? (sorry, I don't know how exactly we attach VS Code vars to our CSS). Does it mean we would have to pick one from the list you provided, @julieg18 ?

@julieg18
Copy link
Contributor

julieg18 commented Aug 8, 2022

so, 1e1e1e is not a variable itself, right? (sorry, I don't know how exactly we attach VS Code vars to our CSS). Does it mean we would have to pick one from the list you provided, @julieg18 ?

Correct! Though I've started testing and I don't think we'll be able to use 1e1e1e as a border color either :( All of the variables mentioned above turn into a white-like color in common white themes which is not visible at all in a white theme... Looking into some other color options that would work better!

@maxagin
Copy link
Contributor Author

maxagin commented Aug 8, 2022

@julieg18 is this all we can use for the default dark theme or there are some more options? Thanks!

image

@julieg18
Copy link
Contributor

julieg18 commented Aug 8, 2022

@julieg18 is this all we can use for the default dark theme or there are some more options? Thanks!

Well, those are all the main variables that are commonly used for borders. While we could technically use a different variable and color (each theme has hundreds of variables), it's preferable that we use a border variable for the table borders since a border variable will most likely work best across all different kind of themes.

@maxagin
Copy link
Contributor Author

maxagin commented Aug 8, 2022

@julieg18 is there more variables for borders that darker than --vscode-tab-border: #252526; ? Thanks

@julieg18
Copy link
Contributor

julieg18 commented Aug 8, 2022

@julieg18 is there more variables for borders that darker than --vscode-tab-border: #252526; ? Thanks

I think that's the darkest border variable!

@shcheklein
Copy link
Member

I think we can always take a var value and also adjust it (e.g. make 10% darker). It can become too complicated with the different themes though, right? (e.g. on a light scheme we might want to make 10% lighter instead). We can try to come up with a formula that works though reasonably well ...

@maxagin
Copy link
Contributor Author

maxagin commented Aug 8, 2022

I think we can always take a var value and also adjust it (e.g. make 10% darker). It can become too complicated with the different themes though, right? (e.g. on a light scheme we might want to make 10% lighter instead). We can try to come up with a formula that works though reasonably well ...

Yes, but it is better to use existing variables. I will find the solution. Thanks

@maxagin
Copy link
Contributor Author

maxagin commented Aug 16, 2022

Hey folks! I have updated the Figma, fixed styles and added some more related requirements. Please review and comment. Thanks!

The updated TODO list:

Rest state

  1. Add a bottom line to every row and remove even rows BG style
  2. Keep the header BG colour the same as the table body BG colour
  3. Use the same line style for header item separators
  4. Keep the caret the same colour as the other icons on the left side

Hover state

  1. Show hover styles (column, row) when the cell is on hover
  2. "lr" on hover. Also, show the hover style for the column
  3. "global" on hover. Also show the hover style for the "seed", "lr", "weight_decay" and three columns
  4. The first column cell on hover. Checkbox and Star rest styles (same like without hover)

Resize columns

  1. When resizing the cell sideline grows from 1px to 2xp and changes colour
  2. When resizing the hover styles are applied the same way as if the block is on hover.
  3. The resize is always applied to the right side of the column
  4. The resize is activated only in the header of the table when hovering on the sidelines

Select and star EXP/s

  1. When a selected row is on hover, add a shadow to show the hovered row
  2. New (lighter) color for the selected row background. One of the reasons: we do not want to have it similar to the color that is used for the buttons
  3. Selected rows have different bottom borders and left/right for selected cell color

Star and checkbox behaviour

  1. Star
  2. Checkbox

@julieg18
Copy link
Contributor

  1. Show hover styles (column, row) when the cell is on hover
  2. "lr" on hover. Also, show the hover style for the column
  3. "global" on hover. Also show the hover style for the "seed", "lr", "weight_decay" and three columns

Didn't we decide to avoid adding column styles on hover due to it causing a lot of rerendering on the last demo meeting?

@maxagin
Copy link
Contributor Author

maxagin commented Aug 16, 2022

Show hover styles (column, row) when the cell is on hover
"lr" on hover. Also, show the hover style for the column
"global" on hover. Also show the hover style for the "seed", "lr", "weight_decay" and three columns

Didn't we decide to avoid adding column styles on hover due to it causing a lot of rerendering on the last demo meeting?

@sroy3 didn't we planed to investigate this, before making any decisions? I am wondering how Weights & Biases managing this.

@sroy3
Copy link
Contributor

sroy3 commented Aug 16, 2022

Show hover styles (column, row) when the cell is on hover
"lr" on hover. Also, show the hover style for the column
"global" on hover. Also show the hover style for the "seed", "lr", "weight_decay" and three columns

Didn't we decide to avoid adding column styles on hover due to it causing a lot of rerendering on the last demo meeting?

@sroy3 didn't we planed to investigate this, before making any decisions? I am wondering how Weights & Biases managing this.

We can maybe do something like this, but style would be very limited as we would just place a huge box over the selected column (no style breaks between cells). The second we start calculating or adding stuff into Redux, that would make it not worth it, unless we virtualize the table rows and columns.

@maxagin
Copy link
Contributor Author

maxagin commented Aug 16, 2022

@sroy3 great! So we can give it a try 👍 Yes?

@maxagin
Copy link
Contributor Author

maxagin commented Aug 16, 2022

Or something like this:

Screen.Recording.2022-08-16.at.6.24.14.PM.mov

Here is the Figma example:

Screen Shot 2022-08-16 at 6 25 46 PM

@sroy3
Copy link
Contributor

sroy3 commented Aug 16, 2022

@julieg18 if you want we can look at this during or next pairing session (we can move the meeting ahead also if you're not sure what else to do.

@maxagin maxagin mentioned this issue Aug 23, 2022
27 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: experiments Area: experiments table webview and everything related priority-p2 Future feature, less priority for now
Projects
None yet
Development

No branches or pull requests

4 participants