gravity-ui / rfc

Gravity RFC is a process for proposing and implementing changes in our ecosystem
MIT License
3 stars 0 forks source link

Solid color for --g-color-base-selection token #10

Open IsaevAlexandr opened 3 months ago

IsaevAlexandr commented 3 months ago

in dnd scenarios, the selected elements should not have a transparent background. I suggest to add --g-color-base-selection-solid css custom property for this case without transparency

https://github.com/gravity-ui/rfc/assets/27916444/a8e5732a-63fe-49a6-afb5-c84dfa5c3a6c

SeqviriouM commented 3 months ago

@amje @korvin89 what do you think?

korvin89 commented 3 months ago

@amje @korvin89 what do you think?

Hey! Sounds good. We should to talk with our designers

IsaevAlexandr commented 3 months ago

And where is a bug with popup bg color only in bark theme with --g-color-base-simple-hover-solid variable cos it has identical styles for bg colors and elements merge in view

https://github.com/gravity-ui/uikit/blob/main/styles/themes/dark/base.scss#L11 and https://github.com/gravity-ui/uikit/blob/main/styles/themes/dark/base.scss#L73 --g-color-base-float variable used for Popup bg color

https://github.com/gravity-ui/rfc/assets/27916444/50ca787b-803d-4972-8d2f-c043000e3702

ykamendrovskiy commented 3 months ago

Hey! Sounds good. We should to talk with our designers

@korvin89 solid colors for selection highlightnig (normal and hover) were removed when upgrading from UIKit 4 to 5 if I remember it correctly

And where is a bug with popup bg color only in bark theme with --g-color-base-simple-hover-solid variable cos it has identical styles for bg colors and elements merge in view

regarding this particular case we have following options:

  1. Create a UIKit-level color variable. Not sure about this one as it seems to be required only in this particular case.
  2. Create a component-level variable with the same values in light themes and adjusted values in dark themes. Something like __--__color-list-selection-drag

@IsaevAlexandr @amje

amje commented 2 months ago

@IsaevAlexandr @korvin89

What if inside Popup we add these styles:

--_--background-color: var(--g-popup-background-color, var(--g-color-base-float));
// ...
--g-color-base-background: var(--_--background-color);

It's like user themization but it's done by component itself. So all components receive right color when addressing --g-color-base-background variable. Then we can apply this technique:

.dnd-item {
  background: linear-gradient(
        to right,
        var(--g-color-base-selection),
        var(--g-color-base-selection)
    ),
    linear-gradient(
        to right,
        var(--g-color-base-background),
        var(--g-color-base-background)
    );
}
SeqviriouM commented 1 month ago

@IsaevAlexandr @korvin89 what do you think about Andrey proposal?

IsaevAlexandr commented 1 month ago

@IsaevAlexandr @korvin89

What if inside Popup we add these styles:

--_--background-color: var(--g-popup-background-color, var(--g-color-base-float));
// ...
--g-color-base-background: var(--_--background-color);

It's like user themization but it's done by component itself. So all components receive right color when addressing --g-color-base-background variable. Then we can apply this technique:

.dnd-item {
  background: linear-gradient(
        to right,
        var(--g-color-base-selection),
        var(--g-color-base-selection)
    ),
    linear-gradient(
        to right,
        var(--g-color-base-background),
        var(--g-color-base-background)
    );
}

I think this is a good solution!

it may be worth applying this approach to all components in gravityui/uikit?

SeqviriouM commented 2 weeks ago

@IsaevAlexandr are you planning to make this change? Or is it better to find someone to solve this issue?