Decathlon / vitamin-web

Decathlon Design System UI components for web applications
https://decathlon.github.io/vitamin-web
Apache License 2.0
282 stars 76 forks source link

feat(`@vtmn/css`, `@vtmn/svelte`, `@vtmn/react`, `@vtmn/vue`): `VtmnChip` variant `input` match now the figma specs #1456

Closed Tlahey closed 1 year ago

Tlahey commented 1 year ago

Changes description

https://github.com/Decathlon/vitamin-web/issues/1455

Expected behaviour image Source : https://www.decathlon.design/726f8c765/p/08dbe1-chip/b/50988a

Context

Today, the chip have to be "selected" in order to display the cancel button image

This PR aim to disable the selected on the variant input and by default, displayed the cancel button

Checklist

Does this introduce a breaking change?

lauthieb commented 1 year ago

Hi @Tlahey, thanks for this contribution! Can you please confirm that this PR will not cause any breaking change and that we can bump packages with a minor release for each of them? Thanks 🙏

Tlahey commented 1 year ago

Hello @lauthieb This enhancement will change the behaviour of the VpChip (caus' the select on the input disappears)

We can put a breaking change on the design but the input properties doesn't changed on the component. The changes are only scoped on the "input" variant

Thus, for me, I think apply a breaking change (for the design part) is a good choice.

lauthieb commented 1 year ago

Thanks @Tlahey for this explanation. What do you mean about applying a BREAKING CHANGE for design? You mean bump to major release your changes? Or just in Figma with a communication? Thank you

Tlahey commented 1 year ago

Hello @lauthieb Because the design now match the figma's version, the user who used the VtmnChip with a input variant will have changes when it will upgrade the version.

Selected input is no longer available on this variant The color of the variant stay white, and no longer has the color blue. So the design will changed for the application.

lauthieb commented 1 year ago

Hi @Tlahey, Ok, thank you. So, I will merge it with a "BREAKING CHANGE" :)