strapi / design-system

Strapi.io's design system :rocket:
https://design-system.strapi.io
MIT License
472 stars 166 forks source link

fix: button icons position and width #1779

Closed HichamELBSI closed 2 months ago

HichamELBSI commented 2 months ago

What does it do?

Fix the alignment of the start and end icons.

Why is it needed?

Describe the issue you are solving.

How to test it?

Provide information about the environment and the path to verify the behaviour.

Related issue(s)/PR(s)

https://github.com/strapi/strapi/issues/20986

vercel[bot] commented 2 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
design-system ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 29, 2024 9:41am
changeset-bot[bot] commented 2 months ago

⚠️ No Changeset found

Latest commit: 4103348b407fe21645661584d5c401e22463dbb5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

HichamELBSI commented 2 months ago

Thanks for the PR. Aren't we missing a margin right & left for start & end icons now ?

If this fix is what we wanted to do visually (cc @lucasboilly), there is no need to add a margin, there is a 8px gap between icons and the text. Now that the text is taking 100% of the remaining space, the text will be centered by default. I don't understand why there was a flex 1 on the icons (cc @simotae14 @remidej if you have any context about it 🤷🏽‍♂️ ) ⬇️ Screenshots of the use cases with the new update.

Start icon - normal size Screenshot 2024-08-29 at 11 13 10

Start icon - fullwidth (which is used in the CM list view filters) Screenshot 2024-08-29 at 11 13 06

End icon - normal size Screenshot 2024-08-29 at 11 15 25 Screenshot 2024-08-29 at 11 15 30

Start + End icon Screenshot 2024-08-29 at 11 12 52 Screenshot 2024-08-29 at 11 12 58

alexandrebodin commented 2 months ago

From @lucasboilly comment here https://www.notion.so/strapi/The-button-in-the-filters-of-the-list-view-is-broken-029d9bac6fa64046ae56782fce9496c0?d=7eb956cf8eea434199d2dfcf6483ca65 I think we are expecting the icon to always be attached to the text even in fullwidth so I think we have to remove the flex & not sure the gap will still work but I let you handle that :D

HichamELBSI commented 2 months ago

From @lucasboilly comment here https://www.notion.so/strapi/The-button-in-the-filters-of-the-list-view-is-broken-029d9bac6fa64046ae56782fce9496c0?d=7eb956cf8eea434199d2dfcf6483ca65 I think we are expecting the icon to always be attached to the text even in fullwidth so I think we have to remove the flex & not sure the gap will still work but I let you handle that :D

You're right, my bad. It should be good now, the gap seems to be working properly. Screenshot 2024-08-29 at 11 28 56 Screenshot 2024-08-29 at 11 28 52 Screenshot 2024-08-29 at 11 28 45 Screenshot 2024-08-29 at 11 28 40 Screenshot 2024-08-29 at 11 28 36 Screenshot 2024-08-29 at 11 28 23

lucasboilly commented 2 months ago

It looks great, thanks @HichamELBSI!

alexandrebodin commented 2 months ago

@HichamELBSI I think you need to add a changeset so this can trigger a new release ?