fix: preserve opacity values in multiple shadows using color-mix#19819
fix: preserve opacity values in multiple shadows using color-mix#19819buyuan-dev wants to merge 2 commits intotailwindlabs:mainfrom
Conversation
WalkthroughThe shadow color replacement utility was updated to handle alpha channels robustly. New helpers extract and strip alpha from color strings and detect whether a replacement expression preserves alpha. replaceShadowColors now operates on a base color without alpha, preserves original alphas by wrapping replacements with a color-mix when needed, and rounds fractional alpha percentages to avoid precision issues. A suite of tests was added to validate behavior across rgba/hsla forms, percentage alphas, multiple shadows, and modifier interactions. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
![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/utils/replace-shadow-colors.ts (1)
123-124: Consider usingin oklabfor consistency withwithAlpha.The existing
withAlphafunction inutilities.tsusescolor-mix(in oklab, ...)for perceptually uniform blending. Usingin srgbhere may produce subtly different blending results. Consider aligning with the existing pattern unless there's a specific reason to prefer sRGB.♻️ Suggested change
- replacementColor = `color-mix(in srgb, transparent, ${replacementColor} ${alpha})` + replacementColor = `color-mix(in oklab, transparent, ${replacementColor} ${alpha})`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tailwindcss/src/utils/replace-shadow-colors.ts` around lines 123 - 124, Change the color-mix space from "in srgb" to "in oklab" when constructing replacementColor with alpha in replace-shadow-colors.ts so it matches the perceptual blending used by withAlpha in utilities.ts; specifically update the branch that sets replacementColor = `color-mix(in srgb, transparent, ${replacementColor} ${alpha})` to use `in oklab` instead, preserving the same template and alpha handling and only altering the color space identifier.
🤖 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/utils/replace-shadow-colors.ts`:
- Around line 28-38: The legacy comma-alpha detection (/,\s*([\d.]+)\s*\)/)
incorrectly captures any trailing numeric component; update the logic in
replace-shadow-colors to only treat comma syntax as an alpha when the color
function is explicitly rgba or hsla and there are four comma-separated
components: first check that the string starts with "rgba(" or "hsla(" (e.g.,
color.startsWith("rgba(") || color.startsWith("hsla(")), extract the
parenthesized contents, split by commas and trim, verify the resulting array has
length 4, then parse the fourth item as the alpha (replacing the current
commaAlphaMatch block that uses /,\s*([\d.]+)\s*\)/ with this stricter
validation before computing and returning the rounded percentage from alphaNum).
- Around line 58-64: The regex in replace-shadow-colors.ts is accidentally
matching non-alpha forms (rgb/hsl); update the pattern used for commaMatch to
require the alpha form (match only "rgba" or "hsla") and correctly capture the
color components before the final alpha value so rgb/hsl inputs are ignored;
specifically adjust the regex that defines commaMatch (and its subsequent use of
commaMatch[1] and commaMatch[2]) so it only matches /(rgba|hsla)\(...\)/i (or an
equivalent pattern that ensures the trailing alpha is present) and then return
the reconstructed color using the captured non-alpha values as before.
---
Nitpick comments:
In `@packages/tailwindcss/src/utils/replace-shadow-colors.ts`:
- Around line 123-124: Change the color-mix space from "in srgb" to "in oklab"
when constructing replacementColor with alpha in replace-shadow-colors.ts so it
matches the perceptual blending used by withAlpha in utilities.ts; specifically
update the branch that sets replacementColor = `color-mix(in srgb, transparent,
${replacementColor} ${alpha})` to use `in oklab` instead, preserving the same
template and alpha handling and only altering the color space identifier.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 77f75b2a-daed-47bc-8ca8-9909b8d4a245
📒 Files selected for processing (1)
packages/tailwindcss/src/utils/replace-shadow-colors.ts
| // Legacy rgba/hsla syntax with comma: rgba(0, 0, 0, 0.12) | ||
| const commaAlphaMatch = color.match(/,\s*([\d.]+)\s*\)/) | ||
| if (commaAlphaMatch) { | ||
| const alpha = commaAlphaMatch[1] | ||
| const alphaNum = parseFloat(alpha) | ||
| if (!isNaN(alphaNum)) { | ||
| // Round to avoid floating point precision issues | ||
| return `${Math.round(alphaNum * 100)}%` | ||
| } | ||
| return null | ||
| } |
There was a problem hiding this comment.
Legacy comma regex incorrectly matches non-alpha colors.
The regex /,\s*([\d.]+)\s*\)/ matches the last comma-separated value in any color, not just alpha channels. For rgb(255, 128, 64) (no alpha), this matches , 64) and extracts 6400% as alpha. Similarly, rgb(0, 0, 0) would extract 0%.
This causes downstream issues: stripAlpha would produce invalid rgb(255, 128), and the output would be wrapped with a bogus alpha percentage.
The fix should verify the color function explicitly requires an alpha channel (i.e., is rgba or hsla with 4 comma-separated values).
🐛 Proposed fix
- // Legacy rgba/hsla syntax with comma: rgba(0, 0, 0, 0.12)
- const commaAlphaMatch = color.match(/,\s*([\d.]+)\s*\)/)
+ // Legacy rgba/hsla syntax with comma: rgba(0, 0, 0, 0.12)
+ // Must be rgba/hsla (with 'a') to have 4 values where the last is alpha
+ const commaAlphaMatch = color.match(/^(?:rgba|hsla)\([^,]+,[^,]+,[^,]+,\s*([\d.]+)\s*\)$/i)
if (commaAlphaMatch) {📝 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.
| // Legacy rgba/hsla syntax with comma: rgba(0, 0, 0, 0.12) | |
| const commaAlphaMatch = color.match(/,\s*([\d.]+)\s*\)/) | |
| if (commaAlphaMatch) { | |
| const alpha = commaAlphaMatch[1] | |
| const alphaNum = parseFloat(alpha) | |
| if (!isNaN(alphaNum)) { | |
| // Round to avoid floating point precision issues | |
| return `${Math.round(alphaNum * 100)}%` | |
| } | |
| return null | |
| } | |
| // Legacy rgba/hsla syntax with comma: rgba(0, 0, 0, 0.12) | |
| // Must be rgba/hsla (with 'a') to have 4 values where the last is alpha | |
| const commaAlphaMatch = color.match(/^(?:rgba|hsla)\([^,]+,[^,]+,[^,]+,\s*([\d.]+)\s*\)$/i) | |
| if (commaAlphaMatch) { | |
| const alpha = commaAlphaMatch[1] | |
| const alphaNum = parseFloat(alpha) | |
| if (!isNaN(alphaNum)) { | |
| // Round to avoid floating point precision issues | |
| return `${Math.round(alphaNum * 100)}%` | |
| } | |
| return null | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/tailwindcss/src/utils/replace-shadow-colors.ts` around lines 28 -
38, The legacy comma-alpha detection (/,\s*([\d.]+)\s*\)/) incorrectly captures
any trailing numeric component; update the logic in replace-shadow-colors to
only treat comma syntax as an alpha when the color function is explicitly rgba
or hsla and there are four comma-separated components: first check that the
string starts with "rgba(" or "hsla(" (e.g., color.startsWith("rgba(") ||
color.startsWith("hsla(")), extract the parenthesized contents, split by commas
and trim, verify the resulting array has length 4, then parse the fourth item as
the alpha (replacing the current commaAlphaMatch block that uses
/,\s*([\d.]+)\s*\)/ with this stricter validation before computing and returning
the rounded percentage from alphaNum).
| // Legacy rgba/hsla syntax with comma: rgba(0, 0, 0, 0.12) or hsla(0, 0%, 0%, 0.3) | ||
| const commaMatch = color.match(/^(rgba?|hsla?)\(([\d\s.,%]+),\s*[\d.]+\s*\)$/i) | ||
| if (commaMatch) { | ||
| const type = commaMatch[1].toLowerCase().replace('a', '') | ||
| const values = commaMatch[2].trim() | ||
| return `${type}(${values})` | ||
| } |
There was a problem hiding this comment.
Same regex issue: matches non-alpha rgb/hsl colors.
The regex rgba?|hsla? matches rgb and hsl (not just rgba/hsla). For rgb(255, 128, 64), this would capture 255, 128 and return rgb(255, 128) — an invalid color.
🐛 Proposed fix
- // Legacy rgba/hsla syntax with comma: rgba(0, 0, 0, 0.12) or hsla(0, 0%, 0%, 0.3)
- const commaMatch = color.match(/^(rgba?|hsla?)\(([\d\s.,%]+),\s*[\d.]+\s*\)$/i)
+ // Legacy rgba/hsla syntax with comma: rgba(0, 0, 0, 0.12) or hsla(0, 0%, 0%, 0.3)
+ // Only match 4-value colors (rgba/hsla with explicit alpha)
+ const commaMatch = color.match(/^(rgba|hsla)\(([^,]+,[^,]+,[^,]+),\s*[\d.]+\s*\)$/i)
if (commaMatch) {
const type = commaMatch[1].toLowerCase().replace('a', '')
const values = commaMatch[2].trim()📝 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.
| // Legacy rgba/hsla syntax with comma: rgba(0, 0, 0, 0.12) or hsla(0, 0%, 0%, 0.3) | |
| const commaMatch = color.match(/^(rgba?|hsla?)\(([\d\s.,%]+),\s*[\d.]+\s*\)$/i) | |
| if (commaMatch) { | |
| const type = commaMatch[1].toLowerCase().replace('a', '') | |
| const values = commaMatch[2].trim() | |
| return `${type}(${values})` | |
| } | |
| // Legacy rgba/hsla syntax with comma: rgba(0, 0, 0, 0.12) or hsla(0, 0%, 0%, 0.3) | |
| // Only match 4-value colors (rgba/hsla with explicit alpha) | |
| const commaMatch = color.match(/^(rgba|hsla)\(([^,]+,[^,]+,[^,]+),\s*[\d.]+\s*\)$/i) | |
| if (commaMatch) { | |
| const type = commaMatch[1].toLowerCase().replace('a', '') | |
| const values = commaMatch[2].trim() | |
| return `${type}(${values})` | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/tailwindcss/src/utils/replace-shadow-colors.ts` around lines 58 -
64, The regex in replace-shadow-colors.ts is accidentally matching non-alpha
forms (rgb/hsl); update the pattern used for commaMatch to require the alpha
form (match only "rgba" or "hsla") and correctly capture the color components
before the final alpha value so rgb/hsl inputs are ignored; specifically adjust
the regex that defines commaMatch (and its subsequent use of commaMatch[1] and
commaMatch[2]) so it only matches /(rgba|hsla)\(...\)/i (or an equivalent
pattern that ensures the trailing alpha is present) and then return the
reconstructed color using the captured non-alpha values as before.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/tailwindcss/src/utils/replace-shadow-colors.test.ts (1)
74-79: Add a legacy commahsla(...)test case for completeness.You covered slash
hsla(...), but not comma-formhsla(h, s, l, a). Since legacy syntax support is in scope, one explicit test would close that gap and prevent regressions.Proposed test addition
+ it('should preserve alpha from hsla color with legacy syntax', () => { + let parsed = replaceShadowColors('hsla(0, 0%, 0%, 0.3) 0px 1px 2px', replacer) + expect(parsed).toMatchInlineSnapshot( + `"color-mix(in srgb, transparent, var(--tw-shadow-color, hsl(0, 0%, 0%)) 30%) 0px 1px 2px"`, + ) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tailwindcss/src/utils/replace-shadow-colors.test.ts` around lines 74 - 79, Add a unit test to replace-shadow-colors.test.ts that covers the legacy comma hsla(...) syntax: call replaceShadowColors with a comma-form color like "hsla(0, 0%, 0%, 0.3) 0px 1px 2px" (using the existing replacer) and assert the expected output mirrors the other hsla test (preserving alpha in color-mix with var(--tw-shadow-color)). This ensures replaceShadowColors handles both slash and legacy comma hsla formats.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/tailwindcss/src/utils/replace-shadow-colors.test.ts`:
- Around line 74-79: Add a unit test to replace-shadow-colors.test.ts that
covers the legacy comma hsla(...) syntax: call replaceShadowColors with a
comma-form color like "hsla(0, 0%, 0%, 0.3) 0px 1px 2px" (using the existing
replacer) and assert the expected output mirrors the other hsla test (preserving
alpha in color-mix with var(--tw-shadow-color)). This ensures
replaceShadowColors handles both slash and legacy comma hsla formats.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3c9a9e17-46c0-4ce7-b2ef-2c23f0a409d7
📒 Files selected for processing (1)
packages/tailwindcss/src/utils/replace-shadow-colors.test.ts
1 participant
Summary
Fixes an issue where opacity values were lost when using multiple shadows with different opacity levels. The fix uses
color-mix(in srgb, ...)to properly preserve individual opacity values while still allowing shadow color customization via--tw-shadow-color.Changes
replaceShadowColorsutility to detect alpha channels in shadow colorsrgba(0 0 0 / 0.12)), the fix extracts the alpha and wraps the color replacement incolor-mix(in srgb, transparent, <color> <alpha>)formatTesting
Fixes #19817