Skip to content

Comments

fix: add workaround for safari + position: relative on <tr>#1579

Merged
serhalp merged 2 commits intonpmx-dev:mainfrom
RYGRIT:fix/table-row-click
Feb 23, 2026
Merged

fix: add workaround for safari + position: relative on <tr>#1579
serhalp merged 2 commits intonpmx-dev:mainfrom
RYGRIT:fix/table-row-click

Conversation

@RYGRIT
Copy link
Contributor

@RYGRIT RYGRIT commented Feb 22, 2026

🔗 Linked issue

Fixes: #1554

🧭 Context

Safari doesn't support position: relative on <tr> elements

📚 Description

@vercel
Copy link

vercel bot commented Feb 22, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs.npmx.dev Ready Ready Preview, Comment Feb 22, 2026 4:57pm
npmx.dev Ready Ready Preview, Comment Feb 22, 2026 4:57pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
npmx-lunaria Ignored Ignored Feb 22, 2026 4:57pm

Request Review

@codecov
Copy link

codecov bot commented Feb 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@RYGRIT RYGRIT changed the title fix: Safari doesn't support position: relative on <tr> elements fix(ui): Safari doesn't support position: relative on <tr> elements Feb 22, 2026
@RYGRIT RYGRIT changed the title fix(ui): Safari doesn't support position: relative on <tr> elements fix: safari doesn't support position: relative on <tr> elements Feb 22, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 22, 2026

📝 Walkthrough

Walkthrough

The change modifies the app/components/Package/TableRow.vue file by adding two Tailwind CSS utility classes—transform and scale-100—to the <tr> element's class attribute. This represents a single-line modification with no structural or logic changes to the component. The adjustment addresses the table row interaction issue reported in the linked issue.

Suggested reviewers

  • danielroe
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description references a specific issue (#1554) and provides relevant context about Safari's lack of support for position: relative on tr elements, which aligns with the code changes.
Linked Issues check ✅ Passed The code change adds Tailwind CSS classes (transform and scale-100) to address Safari's tr positioning issue, which directly resolves the UI bug described in issue #1554 where clicks were misrouted.
Out of Scope Changes check ✅ Passed The changes are scoped solely to the TableRow component styling to fix the Safari tr click bug; no unrelated modifications are present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
app/components/Package/TableRow.vue (1)

47-47: The transform class is redundant alongside scale-100.

In UnoCSS preset-wind4, scale-100 emits the CSS scale property directly and does not depend on the transform utility. The transform utility sets a separate transform: property for rotate and skew variables. Having both together adds unnecessary class noise without functional benefit.

Consider removing transform and keeping only scale-100.

<template>
<tr
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"
class="group relative transform scale-100 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"
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 22, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Consider adding clip-path: inset(0) to fully contain the last row's overlay in Safari.

The CSS transform correctly establishes a containing block on <tr> in Safari (where position: relative is ignored), which is the right approach. However, the blog post referenced in the PR description explicitly notes that after adding the transform, "the last link's clickable area expands below the table" in Safari — i.e. the last row's ::after overlay can still overflow the table's bottom edge. The complete solution recommended in that post uses transform: translate(0) alongside clip-path: inset(0) on tr.

The existing focus-visible:ring-inset on the <tr> already draws the ring inward, so it remains fully visible under clip-path: inset(0).

🛡️ Proposed addition
- class="group relative transform scale-100 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 transform scale-100 [clip-path:inset(0)] 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"

Or if a scoped CSS rule is preferred (mirrors the existing <style scoped> block):

 tr {
   transform: scale(1);
+  clip-path: inset(0);
 }
📝 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="group relative transform scale-100 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 transform scale-100 [clip-path:inset(0)] 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"

Copy link
Member

Choose a reason for hiding this comment

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

what do you think of this comment @RYGRIT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what do you think of this comment @RYGRIT?

I tested it locally and didn't encounter the overflow issue mentioned in the blog post (I haven't figured out why it didn't occur). However, I agree with adding [clip-path:inset(0)] as a defense. WebKit's handling of table borders and absolute positioning can differ across Safari versions, so explicitly clipping the overlay ensures it never goes off-line. I also removed the redundant transform class.

@danielroe danielroe changed the title fix: safari doesn't support position: relative on <tr> elements fix: add workaround for safari + position: relative on <tr> Feb 22, 2026
@serhalp serhalp added this pull request to the merge queue Feb 23, 2026
Merged via the queue into npmx-dev:main with commit 975bb0a Feb 23, 2026
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UI bug, on package search clicking almost anywhere opens the last package in the table

3 participants