feat(@vtmn/svelte): manage height parameter of `VtmnSkeleton` component

[thollander]

Changes description

unit has non-sense to be managed inside this component, we should instead rely on CSS API instead of managing all possible unit values inside our components. If the value is not correct, CSS will manage it its way.

from <VtmnSkeleton width="100" /> to <VtmnSkeleton width="100%" /> or <VtmnSkeleton width="100px" />

Also add a height parameter which follows this same logic too. <VtmnSkeleton width="100%" height="2em" />

Note: this should be done in other implementations too (React, Vue)

Context

Closes https://github.com/Decathlon/vitamin-web/issues/1403

Checklist

• [x] Make sure you are requesting to pull a topic/feature/bugfix branch. Please, don't request directly from your main!
• [x] Check commits & PR names matches our requested structure. It must follow the https://www.conventionalcommits.org pattern.
• [x] Check your code additions will fail neither code linting checks.
• [x] I have reviewed the submitted code.
• [x] I have tested on related showcases.
• [x] If it includes design changes, please ask for a review design-system-core-team-design GitHub team.

Does this introduce a breaking change?

• Yes

Other information

BREAKING CHANGE: remove unit from VtmnSkeleton, it should now be passed in the width or height directly

[Tlahey]

We've already speak about this issue with @lauthieb and @thibault-mahe .

We've got 2 choices :

• Yours, possibly to add a unit after in order to apply it on the component.
• Add a second input in order to not change the behaviour from the legacy.

For the 1rst solution is possible, but the vision is not introduce breaking changed for other who used vtmn.

But the height is not on the vtmn specification I think 🤔

[thollander]

For the 1rst solution is possible, but the vision is not introduce breaking changed for other who used vtmn.

If the current situation has to be changed, it's the normal life of a library isn't it ? Adding a new pameter such as : height , unitHeight would be very ugly.

[lauthieb]

Hello,

Thank you @thollander for this proposition. Sorry I'm on vacation this week.

Yes, that's the normal life of a library and we were stuck on v0 for month, not allowing us to do breaking changes easily. Sorry I'm on vacation this week but that's now a old story :-)

@Tlahey now that we have bumped to v1.x.x we can use SemVer correctly and we can introduce breaking changes (so a v2.x.x) with changelog + migration guide associated :)

So feel free to propose this, I will let @thibault-mahe accompany you on this week.

(For design specifications, can you speak please with @Decathlon/design-system-core-team-design (@SimonLeclercq, @Sabrinavigil & @MARIEDELATTRE).

Thanks! 🙏

[Tlahey]

Ok for me if we bump to major version and put all the informations necessary on the changelog as breaking change.

But we need to change it also for vue (VtmnSkeleton.vue) + react (VtmnSkeleton.tsx). The behaviour are not up to date since the skeleton's latest version on svelte =/

[thollander]

The first commit has been set as BREAKING (https://github.com/Decathlon/vitamin-web/pull/1404/commits/9e408602718bce763aaecf4fc66cad5ad0632c6a) that will bump accordingly and set the information automatically in changelog.

The question is more, as stated by @lauthieb do we need to create a migration guide ? If so, where do you want to create it and in which format ? Do you guys already have some template in Vitamin for this ?

[lauthieb]

@thollander you're right, as it's mentioned "BREAKING CHANGE:", Lerna will automatically bump to a major release (2.0.0 in this case) and attach the note after "BREAKING CHANGE:" inside the different CHANGELOG.md affected.

@thibault-mahe you're right, we need to bring this modification also in @vtmn/vue & @vtmn/react before merging in order to align our different packages (also with breaking changes).

For the migration guide, we have nothing for now, @thibault-mahe I let you choose the format that you want. Are changelogs enough? Inspiration: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md

Feel free to reach me if you have any further questions.

[lauthieb]

Hi @thollander, with @thibault-mahe, we just merged your PR 🎉

Your changes are now in production in @vtmn/svelte@2.0.0.

The CHANGELOG.md has been generated also with your breaking changes informations: https://github.com/Decathlon/vitamin-web/blob/main/packages/sources/svelte/CHANGELOG.md

Thank you again for this contribution. Enjoy :-)

[thollander]

Hey @lauthieb,

Great news thanks !

I've developed the Vue/React implementation in this PR to get quicker as I now know well the component : https://github.com/Decathlon/vitamin-web/pull/1408

Cheers