fix(ui): implement clickable package table rows#1277
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughModified the TableRow component to expand the clickable area across the entire table row instead of only a small portion of the name cell. The implementation adds a new Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| <style scoped> | ||
| .row-link { | ||
| &::after { | ||
| content: ''; | ||
| position: absolute; | ||
| inset: 0; | ||
| cursor: pointer; | ||
| } | ||
|
|
||
| &:focus-visible::after { | ||
| outline: 2px solid var(--color-fg); | ||
| outline-offset: -2px; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 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 -30Repository: 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=cssRepository: 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.cssRepository: 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.
There was a problem hiding this comment.
I agree on this CSS variable usage. I'm also not finding --color-fg in the Uno CSS config.
There was a problem hiding this comment.
@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:
- Use
--fg- This is the correct variable for foreground color - 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.
| </tr> | ||
| </template> | ||
|
|
||
| <style scoped> |
There was a problem hiding this comment.
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)]
3 participants
fixes: #1192