feat: extract button and link component, unify and improve design#1071
feat: extract button and link component, unify and improve design#1071danielroe merged 100 commits intonpmx-dev:mainfrom
Conversation
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/pages/package/[[org]]/[name].vue (1)
19-19: OnlyLinkBaseis explicitly imported;ButtonBaseandButtonGrouprely on auto-import.
ButtonBase(line 633) andButtonGroup(lines 667, 860) are used in the template without an explicit import, relying on Nuxt's auto-import.LinkBaseis explicitly imported despite also only being used in the template. This is fine functionally but slightly inconsistent — consider either importing all three or removing this import to rely entirely on auto-import.app/assets/main.css (1)
189-196: Remove:-moz-focusringunless supporting Firefox versions older than 85.
:focus-visiblehas been natively supported in Firefox since version 85 (January 2021), and:-moz-focusringis a non-standard, Firefox-only predecessor that is now deprecated. By 2025, all major browsers support:focus-visible(Chrome/Edge 86+, Firefox 85+, Safari 15.4+). Unless the project explicitly requires support for pre-85 Firefox, the:-moz-focusringselector adds noise without providing compatibility benefit. If older browser support is needed, the modern fallback pattern (per MDN) uses@supports not selector(:focus-visible)with:focus, not:-moz-focusring.
|
@danielroe @alexdln pushed the following changes:
The old version didn't make it clear at all you could interact with it. Before non of the buttons looked clickable, now all do. I agree these buttons needs a special design, but I'd also rather make that a separate discussion. This PR aims to make all buttons look like buttons and unify their design. The PR fixed one part of the problem and I think some small regressions are fine, since we are still pre-release. Just trying to keep the scope from creeping here. @danielroe I think I am done … |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/components/Button/Base.vue (1)
61-67:kbdbadge dimensions (w-4 h-4) only accommodate single-character shortcuts.Current usages (e.g.
"d",".","c") fit, but if a multi-character shortcut (e.g."⌘K") is ever passed, it will overflow. Consider usingmin-w-4 h-4 px-1or similar to allow the badge to grow.app/pages/package/[[org]]/[name].vue (1)
19-19: Remove unnecessary explicit import —LinkBaseis auto-imported by Nuxt.
LinkBaseis used only in the template, never in the script section.ButtonBaseandButtonGroupare used in the template without explicit imports, relying on Nuxt's component auto-import. This import should be removed for consistency.Proposed fix
-import { LinkBase } from '#components'
…component # Conflicts: # app/components/Package/Versions.vue
danielroe
left a comment
There was a problem hiding this comment.
thank you for your great work on this ❤️
there are still some issues (oblong buttons, etc.) that that we will need to resolve but I think this can be more effectively done after this is merged. 🙏
) Co-authored-by: Robin <robin.kehl@singular-it.de> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Nathan Knowler <nathan@knowler.dev> Co-authored-by: Daniel Roe <daniel@roe.dev>

This one is quite aggressive. It does some additional work on the button and link components and replaces all buttons and links on the frontpage and package-page.
The basic idea behind this is to have a fixed set of component that can be used and will automatically have the desired designs. This way, if we want to redesign buttons/links later, we don't have to hunt them down in the whole code base. This should improve DX and also ensure Consistent design and a11y across the site.
This currently supports the following:
Additionally this does the following design changes:
The reason behind this is, that before a lot of clickable things where really hard to spot. This does add some visual noise, so I think it might be controversial. My line of thinking here is: Make the UI easy to understand and improve visual noise as a second step, instead of the other way around.
Additionally this aligns the static tag design with the button/link tag design, but ensures it is clear what is clickable and what is not.
Open Todos after this is merged:
I know this is tricky to review since it does changes on multiple levels (code structure, a11y, design etc.), so let me know if it would make sense to split this.