Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/components/Compare/FacetSelector.vue
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,12 @@ function isCategoryNoneSelected(category: string): boolean {
:disabled="facet.comingSoon"
:aria-pressed="isFacetSelected(facet.id)"
:aria-label="facet.label"
class="inline-flex items-center gap-1 px-1.5 py-0.5 font-mono text-xs rounded border transition-colors duration-200 focus-visible:outline-accent/70"
class="gap-1 px-1.5 rounded"
:class="
facet.comingSoon
? 'text-fg-subtle/50 bg-bg-subtle border-border-subtle cursor-not-allowed'
: isFacetSelected(facet.id)
? 'text-fg-muted bg-bg-muted border-border'
? 'text-fg-muted bg-bg-muted'
: 'text-fg-subtle bg-bg-subtle border-border-subtle hover:text-fg-muted hover:border-border'
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Avoid a potential size shift between selected and unselected buttons.

Line 77 removes the border from the selected state while the unselected state still uses border colour classes; if ButtonBase doesn’t enforce a fixed border width, toggling could cause a subtle layout shift. Consider keeping a consistent border width (e.g., transparent border on selected).

Suggested tweak
-              ? 'text-fg-muted bg-bg-muted'
+              ? 'text-fg-muted bg-bg-muted border border-transparent'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
class="gap-1 px-1.5 rounded"
:class="
facet.comingSoon
? 'text-fg-subtle/50 bg-bg-subtle border-border-subtle cursor-not-allowed'
: isFacetSelected(facet.id)
? 'text-fg-muted bg-bg-muted border-border'
? 'text-fg-muted bg-bg-muted'
: 'text-fg-subtle bg-bg-subtle border-border-subtle hover:text-fg-muted hover:border-border'
class="gap-1 px-1.5 rounded"
:class="
facet.comingSoon
? 'text-fg-subtle/50 bg-bg-subtle border-border-subtle cursor-not-allowed'
: isFacetSelected(facet.id)
? 'text-fg-muted bg-bg-muted border border-transparent'
: 'text-fg-subtle bg-bg-subtle border-border-subtle hover:text-fg-muted hover:border-border'

Copy link
Copy Markdown
Member Author

@userquin userquin Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BaseButton has border border-border: html validator error => error Class "border-border" duplicated no-dup-class, and so, CI will fail

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@userquin, thank you for the clarification! You're absolutely right—if ButtonBase already provides border border-border by default, then my suggestion to add border border-transparent would indeed duplicate the border class and fail the HTML validator.

Given that ButtonBase enforces a consistent border width across all states, there's no layout shift concern. The selected state uses the default border color from ButtonBase, while the unselected state simply overrides it with border-border-subtle. Your current implementation is correct.

I apologize for not considering the ButtonBase component's default styles when making my initial suggestion.

🐰


✏️ Learnings added
Learnt from: userquin
Repo: npmx-dev/npmx.dev PR: 1335
File: app/components/Compare/FacetSelector.vue:72-78
Timestamp: 2026-02-10T15:47:33.467Z
Learning: In the npmx.dev project, the `ButtonBase` component (app/components/ButtonBase.vue or similar) provides default `border border-border` styles. When styling ButtonBase instances, avoid adding duplicate `border` classes as this triggers the HTML validator error `no-dup-class` and fails CI.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: skaldebane
Repo: npmx-dev/npmx.dev PR: 760
File: app/components/Compare/FacetCard.vue:134-136
Timestamp: 2026-02-06T07:14:04.242Z
Learning: In `app/components/Compare/FacetCard.vue`, the facet values use `dir="auto"` rather than `dir="ltr"` because the values are not always numbers/sizes and can contain mixed content types.

Learnt from: jellydeck
Repo: npmx-dev/npmx.dev PR: 904
File: app/components/Package/Versions.vue:332-332
Timestamp: 2026-02-04T05:35:00.078Z
Learning: In the npmx.dev project, focus-visible styling for buttons and select elements is applied globally via main.css using the rule: `button:focus-visible, select:focus-visible { outline: 2px solid var(--accent); outline-offset: 2px; border-radius: 4px; }`. Therefore, individual buttons and selects don't need inline focus-visible utility classes like `focus-visible:outline-accent/70` added to them.

Learnt from: jellydeck
Repo: npmx-dev/npmx.dev PR: 904
File: app/components/Package/AccessControls.vue:253-253
Timestamp: 2026-02-04T05:34:26.711Z
Learning: In the npmx.dev project, focus-visible styling for button and select elements is applied globally via app/assets/main.css using `button:focus-visible, select:focus-visible { outline: 2px solid var(--accent); outline-offset: 2px; border-radius: 4px; }`. Individual inline utility classes like `focus-visible:outline-accent/70` are not needed on these elements.

Learnt from: alexdln
Repo: npmx-dev/npmx.dev PR: 838
File: app/pages/package/[...package].vue:445-449
Timestamp: 2026-02-03T13:59:33.392Z
Learning: The copy button pattern in app/pages/package/[...package].vue may be made into a reusable component or pattern in the future, but currently it's acceptable to keep it inline with the CSS-only approach for smooth animations.

"
@click="!facet.comingSoon && toggleFacet(facet.id)"
Expand Down
1 change: 1 addition & 0 deletions nuxt.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ export default defineNuxtConfig({
'/': { prerender: true },
'/200.html': { prerender: true },
'/about': { prerender: true },
'/compare': { prerender: true },
'/privacy': { prerender: true },
'/search': { isr: false, cache: false }, // never cache
'/settings': { prerender: true },
Expand Down
Loading