webstudio-is / webstudio

Open source website builder and Webflow alternative. Webstudio is an advanced visual builder that connects to any headless CMS, supports all CSS properties, and can be hosted anywhere, including with us.
https://webstudio.is
GNU Affero General Public License v3.0
5.42k stars 668 forks source link

Updates to CSSValueListItem - renamed and new variants #2530

Open taylornowotny opened 1 year ago

taylornowotny commented 1 year ago

As always, let me know if I can help with anything.

taylornowotny commented 1 year ago

Figma: https://www.figma.com/file/sfCE7iLS0k25qCxiifQNLE/%F0%9F%93%9A-Webstudio-Library?type=design&node-id=2002%3A6155&mode=design&t=McyS77YjVzwy5uVj-1

TrySound commented 11 months ago

@taylornowotny should I change Label as well? Caz we need now a contrast variant of "default".

taylornowotny commented 11 months ago

@taylornowotny should I change Label as well? Caz we need now a contrast variant of "default".

The "selected" variant will use token: foreground.contrast.main for the Label text and the icon. Is that what you mean? If not, can you please explain your question more?

TrySound commented 11 months ago

So do you suggest to override label styles specifically for list item selected state and label default variant?

TrySound commented 11 months ago

Just looks a like a huge exception just to make primary background. Maybe just use highlighted state here?

taylornowotny commented 11 months ago

The selected state is important to indicate that this item is "applied" (ie. bound). This is necessary in order to choose a variable from the list and see that one item is applied while the other items are not. The highlighted state does not communicate that.

In figma it's not a big deal to change the label text color, just one color override. Another variant of the label is not necessary. It's not even necessary to use a label inside the List Item component - it just needs to use the Label text style.

Use whatever strategy you feel is most appropriate to build this. You can remove the label component from this and replace it with just text if that helps. It will never need a nested icon or a highlight color like other instances of the Label.

TrySound commented 11 months ago

The problem is that we use label not only to render text but also render style variants like set, remote, preset etc. So we need either to override specifically default label styles or reimplement whole label with all those features.

taylornowotny commented 11 months ago

Yea so we don’t use set/preset/etc within a List Item component and that’s why we can just use text instead.

On Mon, Dec 18, 2023 at 2:36 AM Bogdan Chadkin @.***> wrote:

The problem is that we use label not only to render text but also render style variants like set, remote, preset etc. So we need either to override specifically default label styles or reimplement whole label with all those features.

— Reply to this email directly, view it on GitHub https://github.com/webstudio-is/webstudio/issues/2530#issuecomment-1860070134, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUT5FGABBE4GQOPXG5TZEELYKAMDZAVCNFSM6AAAAAA6TC43RWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRQGA3TAMJTGQ . You are receiving this because you were mentioned.Message ID: @.***>

TrySound commented 11 months ago

Ok, then I'm gonna remove such labels support

image