Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
41 changes: 41 additions & 0 deletions app/assets/main.css
Original file line number Diff line number Diff line change
Expand Up @@ -338,3 +338,44 @@ html:has(dialog:modal) {
overflow: hidden;
scrollbar-gutter: stable;
}

/* No-JS Alert Styles */
.noscript-alert {
position: absolute;
top: 58px;
left: 0;
width: 100%;

padding: 0.5rem 1rem;

/* Square and no side borders */
border-radius: 0;
border-top: 0;
border-left: 0;
border-right: 0;
border-bottom: 1px solid #f87171;

font-size: 0.875rem;
line-height: 1.25rem;
text-align: center;
z-index: 40;

/* Default (Dark) */
background-color: rgba(127, 29, 29, 0.2); /* red-900/20 */
color: #fecaca; /* red-200 */
}

@media (min-width: 768px) {
.noscript-alert {
padding: 0.75rem 1rem;
font-size: 1rem;
line-height: 1.5rem;
}
}

/* Light theme override */
:root[data-theme='light'] .noscript-alert {
border-color: #b91c1c; /* red-700 */
background-color: #fef2f2; /* red-50 */
color: #991b1b; /* red-800 */
}
4 changes: 2 additions & 2 deletions app/components/Compare/FacetRow.vue
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,9 @@ function isCellLoading(index: number): boolean {
/>
</template>

<!-- No data -->
<!-- No data (Skeleton) -->
<template v-else-if="!value">
<span class="text-fg-subtle text-sm">-</span>
<SkeletonInline class="w-16 h-4" />
</template>

<!-- Value display -->
Expand Down
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'
Comment on lines +72 to 78
Copy link
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
Contributor 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
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
23 changes: 17 additions & 6 deletions app/composables/usePackageComparison.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,18 @@ export function usePackageComparison(packageNames: MaybeRefOrGetter<string[]>) {
const compactNumberFormatter = useCompactNumberFormatter()
const bytesFormatter = useBytesFormatter()
const packages = computed(() => toValue(packageNames))
const nuxt = useNuxtApp()

// Cache of fetched data by package name (source of truth)
const cache = shallowRef(new Map<string, PackageComparisonData>())

// Derived array in current package order
const packagesData = computed(() => packages.value.map(name => cache.value.get(name) ?? null))
const packagesData = computed<false | (PackageComparisonData | null)[]>(
() =>
import.meta.client &&
!nuxt.isHydrating &&
packages.value.map(name => cache.value.get(name) ?? null),
)

const status = shallowRef<'idle' | 'pending' | 'success' | 'error'>('idle')
const error = shallowRef<Error | null>(null)
Expand Down Expand Up @@ -250,17 +256,22 @@ export function usePackageComparison(packageNames: MaybeRefOrGetter<string[]>) {
// Watch for package changes and refetch (client-side only)
if (import.meta.client) {
watch(
packages,
newPackages => {
fetchPackages(newPackages)
() => [nuxt.isHydrating, packages.value] as const,
([isHydrating, newPackages]) => {
if (!isHydrating) {
fetchPackages(newPackages)
}
},
{ immediate: true },
)
}

// Compute values for each facet
function getFacetValues(facet: ComparisonFacet): (FacetValue | null)[] {
if (!packagesData.value || packagesData.value.length === 0) return []
// If not ready or no data, return array of nulls to render skeletons
if (nuxt.isHydrating || !packagesData.value || packagesData.value.length === 0) {
return Array.from({ length: packages.value.length }, () => null)
}

return packagesData.value.map(pkg => {
if (!pkg) return null
Expand All @@ -277,7 +288,7 @@ export function usePackageComparison(packageNames: MaybeRefOrGetter<string[]>) {

// Check if a facet depends on slow-loading data
function isFacetLoading(facet: ComparisonFacet): boolean {
if (!installSizeLoading.value) return false
if (nuxt.isHydrating || !installSizeLoading.value) return false
// These facets depend on install-size API
return facet === 'installSize' || facet === 'totalDependencies'
}
Expand Down
Loading
Loading