Add safe area utilities for mobile app usage#19921
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51763ed4f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds safe-area helpers and registers utilities that expose safe-area CSS variables and apply them to inset, margin, padding, scroll-margin, scroll-padding, and viewport-height height utilities. Implements a resolver for spacing-like functional values (theme keys, arbitrary values, fractions, multipliers), emits static safe* and safe*-none variables, and wires negative/fractional variants where applicable. Adds tests validating generated CSS snapshots and runtime class handling for both spacing-based and viewport-height safe-area variants. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/tailwindcss/src/utilities.ts (1)
598-625: Extract the spacing resolution path instead of copying it.
resolveSpacingUtilityValuenow reimplements the same theme/bare/fraction resolution thatspacingUtilityalready owns. Any future change to spacing candidate handling will have to be updated in both places, which is an easy drift point for core utilities.Also applies to: 627-666
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tailwindcss/src/utilities.ts` around lines 598 - 625, resolveSpacingUtilityValue duplicates the spacing resolution logic already implemented by spacingUtility; extract that shared resolution path into a single helper (e.g., resolveSpacingCandidate or reuse spacingUtility's internal resolver) and have resolveSpacingUtilityValue call it instead of reimplementing theme.resolve/fraction/bare/multiplier logic. Refactor so the new helper accepts the same inputs used here (candidate, themeKeys, supportsFractions) and internally uses theme.resolve, segment/isPositiveInteger, and isValidSpacingMultiplier to produce the final value; update both resolveSpacingUtilityValue and the original spacingUtility call sites to use the shared helper to avoid duplication and future drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/tailwindcss/src/utilities.test.ts`:
- Around line 319-437: The test currently covers 'safe', 'safe-x-none', and
'safe-b' but misses the primary control utility 'safe-none'; update the
candidate list in the utilities.test.ts test (the array alongside 'safe',
'safe-x-none', 'safe-b') to include 'safe-none' so the snapshot generation also
asserts the .safe-none rule (alongside existing .safe, .safe-x-none, .safe-b)
that sets all --tw-safe-area-* variables to 0; run the test to refresh the
inline snapshot if necessary.
In `@packages/tailwindcss/src/utilities.ts`:
- Around line 775-808: The safe-axis utilities currently use physical sides
(left/right/top/bottom) which breaks logical-axis consistency; update the
entries passed to safeAreaSpacingUtility so that 'inset-x-safe' and any other
"*-x-safe" use the inline-axis pair (inline-start, inline-end) and
'inset-y-safe' and "*-y-safe" use the block-axis pair (block-start, block-end),
and change single-side safe entries (e.g., 'left-safe', 'right-safe',
'top-safe', 'bottom-safe') to their logical equivalents where appropriate
(inline-start/inline-end/block-start/block-end) so the safe variants match
Tailwind’s logical-axis semantics; locate and modify the arrays in the for-loop
that calls safeAreaSpacingUtility (names like 'inset-safe', 'inset-x-safe',
'inset-y-safe', 'top-safe', 'right-safe', 'bottom-safe', 'left-safe') and mirror
the same logical-axis replacement in the other similar blocks noted in the
review.
---
Nitpick comments:
In `@packages/tailwindcss/src/utilities.ts`:
- Around line 598-625: resolveSpacingUtilityValue duplicates the spacing
resolution logic already implemented by spacingUtility; extract that shared
resolution path into a single helper (e.g., resolveSpacingCandidate or reuse
spacingUtility's internal resolver) and have resolveSpacingUtilityValue call it
instead of reimplementing theme.resolve/fraction/bare/multiplier logic. Refactor
so the new helper accepts the same inputs used here (candidate, themeKeys,
supportsFractions) and internally uses theme.resolve, segment/isPositiveInteger,
and isValidSpacingMultiplier to produce the final value; update both
resolveSpacingUtilityValue and the original spacingUtility call sites to use the
shared helper to avoid duplication and future drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6749e52a-559b-41ef-9a78-aab6eaa3f876
⛔ Files ignored due to path filters (1)
packages/tailwindcss/src/__snapshots__/intellisense.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
packages/tailwindcss/src/utilities.test.tspackages/tailwindcss/src/utilities.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 246f79e9ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
riderx
changed the title
|
hey @adamwathan i would love your review on that one, it's easy win |
|
made the missing doc website PR tailwindlabs/tailwindcss.com#2491 |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Confidence Score: 4/5The implementation is functionally correct and well-tested; the main concerns are a duplicated value-resolution helper and a missing set of width viewport utilities rather than any broken behaviour. The CSS custom property fallback mechanism works correctly, all tested class combinations produce the expected output, and invalid combinations are properly rejected. The duplicated resolveSpacingUtilityValue logic is a future maintenance concern rather than a current defect. The resolveSpacingUtilityValue function in utilities.ts and the absence of w-*-safe width viewport utilities are the areas worth a second look. Reviews (1): Last reviewed commit: "Merge branch 'main' into codex/add-safe-..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function resolveSpacingUtilityValue( | ||
| candidate: Extract<Candidate, { kind: 'functional' }>, | ||
| themeKeys: ThemeKey[], | ||
| supportsFractions = false, | ||
| ) { | ||
| if (!candidate.value) return null | ||
|
|
||
| if (candidate.value.kind === 'arbitrary') { | ||
| if (candidate.modifier) return null | ||
| return candidate.value.value | ||
| } | ||
|
|
||
| if (candidate.modifier && (!candidate.value.fraction || !supportsFractions)) return null | ||
|
|
||
| let value = theme.resolve(candidate.value.fraction ?? candidate.value.value, themeKeys) | ||
|
|
||
| if (value === null && supportsFractions && candidate.value.fraction) { | ||
| let [lhs, rhs] = segment(candidate.value.fraction, '/') | ||
| if (!isPositiveInteger(lhs) || !isPositiveInteger(rhs)) return null | ||
| value = `calc(${lhs} / ${rhs} * 100%)` | ||
| } | ||
|
|
||
| if (value === null) { | ||
| let multiplier = theme.resolve(null, ['--spacing']) | ||
| if (multiplier && isValidSpacingMultiplier(candidate.value.value)) { | ||
| value = `calc(${multiplier} * ${candidate.value.value})` | ||
| } | ||
| } | ||
|
|
||
| return value | ||
| } |
There was a problem hiding this comment.
resolveSpacingUtilityValue re-implements the same theme/spacing resolution that already lives inside the handleFunctionalUtility closure of functionalUtility. If the core resolution path is ever extended (e.g., new CSS math keywords, new spacing syntax in isValidSpacingMultiplier), this copy would need to be updated separately, creating a silent drift risk. Consider extracting the shared resolution logic from functionalUtility into a standalone helper that both paths can call.
| for (let [name, property] of [ | ||
| ['h', 'height'], | ||
| ['min-h', 'min-height'], | ||
| ['max-h', 'max-height'], | ||
| ] as const) { | ||
| for (let [viewport, value] of [ | ||
| ['screen', '100vh'], | ||
| ['svh', '100svh'], | ||
| ['lvh', '100lvh'], | ||
| ['dvh', '100dvh'], | ||
| ] as const) { | ||
| staticUtility(`${name}-${viewport}-safe`, [ | ||
| [property, `calc(${value} - (${safeAreaValue('top')} + ${safeAreaValue('bottom')}))`], | ||
| ]) | ||
| } | ||
| } |
There was a problem hiding this comment.
The existing framework already ships w-dvw, w-svw, w-lvw, and w-screen (see lines 1225 and 1247). The PR adds h-*-safe height variants but no equivalent w-dvw-safe, w-svw-safe, w-lvw-safe, or w-screen-safe utilities. On landscape-oriented iOS/iPadOS devices the safe areas appear on the left and right edges, so calc(100dvw - (safe-left + safe-right)) is a common need. Without these utilities users fall back to ad-hoc workarounds — the same situation this PR is trying to solve.
1 participant
Summary
safeandsafe-nonecontrol utilities so safe-area values can be scoped or reset via CSS custom propertiesscreen,svh,lvh, anddvhWhy
Impact
pt-safe-4,-top-safe-px,scroll-p-safe, andh-dvh-safesafe-nonestyle utilities let component subtrees opt out without redefining the spacing helpersValidation
pnpm exec vitest run packages/tailwindcss/src/utilities.test.ts packages/tailwindcss/src/intellisense.test.ts packages/tailwindcss/src/candidate.test.ts packages/tailwindcss/src/prefix.test.ts