feat: tweak button & link styles #1163
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
@jellydeck is attempting to deploy a commit to the serhalp's projects Team on Vercel. A member of the Team first needs to authorize it. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/Package/Dependencies.vue (1)
143-158:⚠️ Potential issue | 🟡 MinorInconsistent focus-visible styling across "Show all" buttons.
This file contains three "Show all" buttons with different focus styling:
- Line 146: Uses the new explicit styling
focus-visible:(outline-2 outline-offset-2 outline-accent)✓- Line 198: Still uses the old
focus-visible:outline-accent/70style- Line 255: Has no focus-visible styling at all
Since the PR removes global focus CSS and standardises inline focus styling, all three buttons should use the same approach. This may relate to the accent colour inconsistencies reported in the PR review comments.
🔧 Proposed fix to standardise focus styling
Apply the same focus-visible classes to the peer dependencies button:
<button v-if="sortedPeerDependencies.length > 10 && !peerDepsExpanded" type="button" - class="mt-2 font-mono text-xs text-fg-muted hover:text-fg transition-colors duration-200 rounded focus-visible:outline-accent/70" + class="my-2 ms-1 font-mono text-xs text-fg-muted hover:text-fg transition-colors duration-200 rounded focus-visible:(outline-2 outline-offset-2 outline-accent)" `@click`="peerDepsExpanded = true" >Apply the same styling to the optional dependencies button:
<button v-if="sortedOptionalDependencies.length > 10 && !optionalDepsExpanded" type="button" - class="mt-2 truncate" + class="my-2 ms-1 font-mono text-xs text-fg-muted hover:text-fg transition-colors duration-200 rounded focus-visible:(outline-2 outline-offset-2 outline-accent)" `@click`="optionalDepsExpanded = true" >Also applies to: 195-210, 252-267
There is a regression here, where the menu links/buttons have their outline back. This should be overwritten (see prod) |
This looks very tight now |
not intended, will do fix once i'll be back on desktop |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/components/Button/Base.vue (1)
8-8: Consider renamingclassicontoiconClassfor consistency.The prop name
classicondeviates from standard camelCase conventions (e.g.,iconClass). This would align better with typical Vue/JS naming patterns and improve readability.♻️ Suggested rename
- classicon?: string + iconClass?: stringAnd update the template reference at line 52:
- <span v-if="classicon" class="size-[1em]" :class="classicon" aria-hidden="true" /> + <span v-if="iconClass" class="size-[1em]" :class="iconClass" aria-hidden="true" />
done 👍 |
done in latest commit 👍 |
4 participants
update the general style of buttons, group buttons and links to be consistent with our design language.
also, removed global css as Input component seems to be done.
Thanks @alexdln @essenmitsosse for feedback, enjoyed working together :)