Skip to content

Conversation

@samejr
Copy link
Member

@samejr samejr commented Jan 12, 2026

Table row divider lines don't render on Safari. This adds a special class for Safari to fix it.

CleanShot 2026-01-12 at 20 49 17@2x

@changeset-bot
Copy link

changeset-bot bot commented Jan 12, 2026

⚠️ No Changeset found

Latest commit: 0374d04

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 12, 2026

Walkthrough

This pull request adds Safari-specific styling support to table components using CSS feature detection. A new .safari-only utility class is introduced in the Tailwind CSS configuration using @supports with the -webkit-hyphens feature query to target Safari browsers. The TableHeader and TableCell components in the React table component are updated to conditionally apply this new class alongside their existing styles, enabling Safari-targeted visual adjustments without modifying the base layer rules.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description is minimal but covers the core issue and includes a screenshot. However, it lacks required template sections like Testing, Changelog details, and the checklist. Complete the PR description template by adding Testing steps, a detailed Changelog entry, and the contribution checklist to meet repository standards.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: adding a Safari-specific fix for missing table row divider lines, which matches the primary objective of the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing touches
  • 📝 Generate docstrings

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.

@claude
Copy link

claude bot commented Jan 12, 2026

PR Review: Fix Safari Table Dividers

Summary

This PR addresses a Safari-specific rendering issue where table row divider lines don't appear. The fix uses a Safari-specific CSS feature detection approach.

Code Quality Assessment

Approach: The solution uses @supports (-webkit-hyphens: none) to detect Safari and applies a border-b border-grid-dimmed fallback for the pseudo-element-based dividers that don't render correctly in Safari.

Observations:

  1. CSS Detection Method
    The -webkit-hyphens: none feature query is a well-known technique for Safari detection. It's reliable and doesn't require JavaScript user-agent sniffing.

  2. Potential Issue - Redundant Class Application ⚠️
    The safari-only class is added to elements unconditionally (both Safari and non-Safari browsers), but the CSS rule only applies styles in Safari via the @supports block. This is harmless but slightly redundant - the class name is perhaps misleading since the class exists on all browsers, only the styles are Safari-specific.

  3. TableHeader Double-handling 🔍
    In TableHeader, there's both:

    • safari-only class (applies border-b in Safari)
    • supports-[(-webkit-hyphens:none)]:after:content-none (hides the pseudo-element in Safari)

    This correctly:

    1. Removes the pseudo-element divider in Safari (since it doesn't render properly)
    2. Adds a real border as fallback via the .safari-only class

    The approach is sound.

  4. TableCell Handling ⚠️
    The safari-only class is added to TableCell, but unlike TableHeader, there's no corresponding supports-[(-webkit-hyphens:none)]:after:content-none to hide the pseudo-element in Safari. This means Safari will potentially show both the (non-rendering) pseudo-element AND the border.

    However, since the pseudo-element doesn't render in Safari anyway, this should be fine in practice. But for consistency and explicit intent, consider adding the supports query to TableCell as well, or adding a comment explaining why it's not needed.

  5. No TableRow Changes 📝
    TableRow also uses a pseudo-element for its divider line (after:absolute after:bottom-0 after:left-3 after:right-0 after:h-px after:bg-grid-dimmed). If the Safari rendering issue affects pseudo-elements on table elements generally, this might also need the same fix. Worth testing to confirm TableRow dividers render correctly in Safari.

Performance Considerations ✅

  • Negligible performance impact - just CSS class additions and a @supports block
  • No JavaScript runtime overhead

Security Concerns ✅

  • None - this is purely presentational CSS

Test Coverage

  • This is a visual/browser-specific fix that's difficult to unit test
  • Manual testing in Safari is appropriate

Recommendations

  1. Minor: Verify TableRow doesn't have the same issue (it uses similar pseudo-element dividers)

  2. Optional: Consider adding a brief comment in the CSS explaining the Safari detection technique for future maintainers:

    /* Safari doesn't render pseudo-element borders on table elements correctly.
       -webkit-hyphens: none is a Safari-only feature used for detection. */
    @supports (-webkit-hyphens: none) {
      .safari-only {
        @apply border-b border-grid-dimmed;
      }
    }

Verdict

Approve with minor suggestions - The fix correctly addresses the Safari rendering issue using a standard browser detection pattern. The implementation is clean and focused.

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: 0

🧹 Nitpick comments (2)
apps/webapp/app/tailwind.css (1)

14-18: Valid Safari detection technique.

Using @supports (-webkit-hyphens: none) is a well-known method to target Safari specifically. The placement outside @layer ensures proper specificity.

Consider adding a brief comment for future maintainers:

📝 Optional: Add explanatory comment
+/* Safari-only styles: -webkit-hyphens is only supported by Safari */
 @supports (-webkit-hyphens: none) {
   .safari-only {
     @apply border-b border-grid-dimmed;
   }
 }
apps/webapp/app/components/primitives/Table.tsx (1)

252-264: Safari fix applied correctly.

Adding safari-only to TableCell provides native border-b in Safari, compensating for the pseudo-element rendering issues. Unlike TableHeader, the cell's pseudo-elements are conditional (hover/focus states) and don't need to be hidden.

One observation: this className string is quite long and dense. Consider extracting the Safari-specific and focus/hover pseudo-element classes into a variable for readability, though this is optional given the existing code style.

♻️ Optional: Extract for readability
const safariAndFocusClasses = [
  "safari-only",
  "has-[[tabindex='0']:focus]:before:absolute has-[[tabindex='0']:focus]:before:-top-px has-[[tabindex='0']:focus]:before:left-0 has-[[tabindex='0']:focus]:before:h-px has-[[tabindex='0']:focus]:before:w-3 has-[[tabindex='0']:focus]:before:bg-grid-dimmed",
  "has-[[tabindex='0']:focus]:after:absolute has-[[tabindex='0']:focus]:after:bottom-0 has-[[tabindex='0']:focus]:after:left-0 has-[[tabindex='0']:focus]:after:right-0 has-[[tabindex='0']:focus]:after:h-px has-[[tabindex='0']:focus]:after:bg-grid-dimmed",
].join(" ");

// Then in the className:
className={cn(
  safariAndFocusClasses,
  "text-xs text-charcoal-400",
  variants[variant].cell,
  // ...rest of classes
)}
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 768206c and 0374d04.

📒 Files selected for processing (2)
  • apps/webapp/app/components/primitives/Table.tsx
  • apps/webapp/app/tailwind.css
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead

Every Trigger.dev task must be exported; use task() function with unique id and run async function

Files:

  • apps/webapp/app/components/primitives/Table.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use zod for validation in packages/core and apps/webapp

Files:

  • apps/webapp/app/components/primitives/Table.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use function declarations instead of default exports

Files:

  • apps/webapp/app/components/primitives/Table.tsx
apps/webapp/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Access all environment variables through the env export of env.server.ts instead of directly accessing process.env in the Trigger.dev webapp

In webapp development, access environment variables via env export from apps/webapp/app/env.server.ts; never use process.env directly

Files:

  • apps/webapp/app/components/primitives/Table.tsx
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

apps/webapp/**/*.{ts,tsx}: When importing from @trigger.dev/core in the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp

Files:

  • apps/webapp/app/components/primitives/Table.tsx
**/*.{js,ts,jsx,tsx,json,md,css,scss}

📄 CodeRabbit inference engine (AGENTS.md)

Format code using Prettier

Files:

  • apps/webapp/app/components/primitives/Table.tsx
  • apps/webapp/app/tailwind.css
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js}: Import from @trigger.dev/core using subpaths only; never import from root of @trigger.dev/core
Always import task definitions from @trigger.dev/sdk, never from @trigger.dev/sdk/v3 or deprecated client.defineJob pattern

Files:

  • apps/webapp/app/components/primitives/Table.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: claude-review
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/webapp/app/components/primitives/Table.tsx (1)

82-86: Sound approach for Safari compatibility.

The dual strategy is correct:

  1. safari-only applies native border-b in Safari via the CSS @supports rule
  2. supports-[(-webkit-hyphens:none)]:after:content-none hides the pseudo-element in Safari to prevent double borders

This maintains the pseudo-element approach for other browsers while using native borders in Safari.

@samejr samejr merged commit 7a94908 into main Jan 12, 2026
59 of 60 checks passed
@samejr samejr deleted the fix(webapp)-safari-table-dividers branch January 12, 2026 21:14
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.

4 participants