Skip to content

fix(ui): implement clickable package table rows #1277

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danielroe merged 1 commit into from
Feb 9, 2026
+20 −4
Merged

fix(ui): implement clickable package table rows #1277

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 20 additions & 4 deletions app/components/Package/TableRow.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@ const allMaintainersText = computed(() => {

<template>
<tr
class="group border-b border-border hover:bg-bg-muted transition-colors duration-200 focus-visible:ring-2 focus-visible:ring-fg focus-visible:ring-inset focus-visible:outline-none focus:bg-bg-muted"
class="group relative border-b border-border hover:bg-bg-muted transition-colors duration-200 focus-visible:ring-2 focus-visible:ring-fg focus-visible:ring-inset focus-visible:outline-none focus:bg-bg-muted"
tabindex="0"
:data-result-index="index"
>
<!-- Name (always visible) -->
<td class="py-2 px-3">
<NuxtLink
:to="packageUrl"
class="font-mono text-sm text-fg hover:text-accent-fallback transition-colors duration-200"
class="row-link font-mono text-sm text-fg hover:text-accent-fallback transition-colors duration-200"
dir="ltr"
>
{{ pkg.name }}
Expand Down Expand Up @@ -111,7 +111,7 @@ const allMaintainersText = computed(() => {
name: '~username',
params: { username: maintainer.username || maintainer.name || '' },
}"
class="hover:text-accent-fallback transition-colors duration-200"
class="relative z-10 hover:text-accent-fallback transition-colors duration-200"
@click.stop
>{{ maintainer.username || maintainer.name || maintainer.email }}</NuxtLink
><span v-if="idx < Math.min(pkg.maintainers.length, 3) - 1">, </span>
Expand All @@ -127,7 +127,7 @@ const allMaintainersText = computed(() => {
<td v-if="isColumnVisible('keywords')" class="py-2 px-3 text-end">
<div
v-if="pkg.keywords?.length"
class="flex flex-wrap gap-1 justify-end"
class="relative z-10 flex flex-wrap gap-1 justify-end"
:aria-label="$t('package.card.keywords')"
>
<ButtonBase
Expand Down Expand Up @@ -198,3 +198,19 @@ const allMaintainersText = computed(() => {
</td>
</tr>
</template>

<style scoped>
Copy link
Contributor

@carwack carwack Feb 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Why are we not using the Tailwind-ish utility classes in line in the template for this?

I do believe this is not a too crazy thing since it only gets used once.

after:content-[''] after:absolute after:inset-0 after:cursor-pointer focus-visible:after:outline-2 focus-visible:after:outline-offset-[-2px] focus-visible:after:outline-[var(--fg)]

Copy link
Member

@danielroe danielroe Feb 9, 2026

Choose a reason for hiding this comment

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

this is also a good point

.row-link {
&::after {
content: '';
position: absolute;
inset: 0;
cursor: pointer;
}

&:focus-visible::after {
outline: 2px solid var(--color-fg);
outline-offset: -2px;
}
}
Comment on lines +202 to +215
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for CSS variable definitions and usage patterns

# Check if --color-fg is defined anywhere
echo "=== Searching for --color-fg definition ==="
rg -n '\-\-color-fg' --type=css --type=vue

echo ""
echo "=== Checking main.css for design tokens ==="
fd -t f 'main.css' -x head -100 {}

echo ""
echo "=== Searching for other --color- or --fg patterns ==="
rg -n '\-\-(color-|fg)' --type=css --type=vue | head -30

Repository: npmx-dev/npmx.dev

Length of output: 3367

🏁 Script executed:

#!/bin/bash
# Check for global focus-visible styles in stylesheets

echo "=== Searching for focus-visible styles in main.css ==="
rg -n 'focus-visible' app/assets/main.css

echo ""
echo "=== Searching for focus-visible styles across the codebase ==="
rg -n 'focus-visible' --type=css

echo ""
echo "=== Checking how ::after elements with outlines are styled elsewhere ==="
rg -n '::after.*outline|outline.*::after' --type=css

Repository: npmx-dev/npmx.dev

Length of output: 313

🏁 Script executed:

#!/bin/bash
# Get context around the focus-visible style in main.css
sed -n '220,235p' app/assets/main.css

Repository: npmx-dev/npmx.dev

Length of output: 486

Use the correct CSS variable --fg instead of undefined --color-fg.

The variable var(--color-fg) does not exist in the design system. Based on main.css, the correct variable is --fg. However, consider using the global focus-visible styling already defined in main.css (:focus-visible { outline: 2px solid var(--accent) !important; }), as inline focus-visible styles on elements are not the project convention. If this ::after pseudo-element requires custom focus styling, ensure it aligns with the global pattern.

Copy link
Contributor

@carwack carwack Feb 9, 2026

Choose a reason for hiding this comment

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

I agree on this CSS variable usage. I'm also not finding --color-fg in the Uno CSS config.

Copy link
Contributor

@coderabbitai coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

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

@carwack, thank you for confirming! The CSS variable --color-fg is indeed undefined and needs to be corrected.

Based on the design system in app/assets/main.css, you have two options:

  1. Use --fg - This is the correct variable for foreground color
  2. Use --accent - This matches the global focus-visible pattern (:focus-visible { outline: 2px solid var(--accent) !important; })

I'd recommend using --accent to align with the project's existing focus-visible styling convention. The fix would be:

.row-link {
  &::after {
    content: '';
    position: absolute;
    inset: 0;
    cursor: pointer;
  }

  &:focus-visible::after {
    outline: 2px solid var(--accent);
    outline-offset: -2px;
  }
}

Would you like me to plan this change for you?

🧠 Learnings used 
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.

Learnt from: jellydeck
Repo: npmx-dev/npmx.dev PR: 904
File: app/components/Package/Versions.vue:332-332
Timestamp: 2026-02-04T05:34:54.335Z
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:20.527Z
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: 1042
File: app/assets/main.css:120-129
Timestamp: 2026-02-06T11:09:26.789Z
Learning: In the npmx.dev project's app/assets/main.css, the `:root[data-theme='light'][data-bg-theme='black']` variant intentionally only overrides `--bg-subtle` and reuses the default light theme values for `--bg`, `--bg-muted`, and `--bg-elevated`. This is by design because the default light theme is mostly neutral, so selective override is sufficient for the black theme variant.

</style>
Loading