feat: extend badge to be available for group links#995
feat: extend badge to be available for group links#995kevinzunigacuellar merged 17 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 856e430 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for astro-starlight ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
size-limit report 📦
|
HiDeoo
left a comment
There was a problem hiding this comment.
More badges 🎉 Amazing.
I left a few questions mostly regarding the documentation and I have some visual feedback as well.
I am not sure if this is expected or part of the initial design, but I think the gap between a label and a badge is different for a link and a group. I am not saying they should be the same, but I personally think the gap is too small for a group at least.
I think there may be some wrapping behavior that is not handled yet compared to links:
Last point, maybe we should also update a few tests to ensure in a snapshot we get badges for groups too?
Impatient to see this merged and add more badges to my documentations ^^
Co-authored-by: HiDeoo <494699+HiDeoo@users.noreply.github.com>
|
One last thing that I would like to get some input. I noticed that the spacing for multi line groups could get a bit wide. This gets more noticeable with a badge. Should we decrease the line height for groups? or does this looks good? 
|
Indeed, this is something I noticed in the past, and I'm pretty sure we talked about this somewhere else but I cannot find where. I am no designer but for what it's worth, I personally think it could be slightly decreased. Not sure what others think though. |
Good catch. I think we did discuss and adjust this for individual link items, but didn’t for group headings. Compare the spacing here for example: 
The trick is to work out how to reduce the line-height and then add back some vertical padding so that the overall spacing for lines that don’t wrap stays the same. #419 did this for link items. |
|
Thanks for the suggestions and the reference PR @delucis . I gave it a try, taking |
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
delucis
left a comment
There was a problem hiding this comment.
Yay! I think this is in pretty good shape — thanks @kevinzunigacuellar.
Few more small details, to clean this up.
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
delucis
left a comment
There was a problem hiding this comment.
Really great work on this one @kevinzunigacuellar. Left one last note to polish up the changeset, but then LGTM! 🌟
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
|
Oops, I guess we better cut a release then :-D Usually we wait before merging changes like this because they immediately update docs even before we release them. I totally did not make that clear with my “LGTM” comment though, sorry 😅 Let me take a look through the open PRs to see if there’s some other stuff we can get out with this. |
…tro#995) Co-authored-by: HiDeoo <494699+HiDeoo@users.noreply.github.com> Co-authored-by: Chris Swithinbank <357379+delucis@users.noreply.github.com>
What kind of changes does this PR include?
Description