fix: preserve opacity values in multiple shadows using color-mix#19819
Open
buyuan-dev wants to merge 3 commits intotailwindlabs:mainfrom
Open
fix: preserve opacity values in multiple shadows using color-mix#19819buyuan-dev wants to merge 3 commits intotailwindlabs:mainfrom
buyuan-dev wants to merge 3 commits intotailwindlabs:mainfrom
Conversation
Contributor
WalkthroughThe shadow color replacement utility was extended with helpers to extract alpha ( 🚥 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 |
Contributor
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
Contributor
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
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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-30: Restore the missing declaration for the regex match by
re-adding the const commaAlphaMatch =
color.match(/^(?:rgba|hsla)\([^,]+,[^,]+,[^,]+,\s*([\d.]+)\s*\)$/i) so the
subsequent if (commaAlphaMatch) branch in replace-shadow-colors.ts can access
the parsed alpha; locate where commaAlphaMatch is currently referenced and
either uncomment or reinsert that exact declaration (using the same regex)
immediately before the if (commaAlphaMatch) check to fix the undefined
identifier and restore type-checking.
- Around line 10-23: extractAlpha() is using the wrong capture index for the
"slash" regex and stripAlpha() has a mismatched pattern so modern hsl/rgba with
unit tokens (like "deg") or legacy percentage alphas (e.g. "rgba(..., 12%)" /
"hsla(..., 30%)") are not handled consistently; update extractAlpha() to pull
the alpha from slashAlphaMatch[3] (the third capture in
/^(rgba?|hsla?)\((.+)\s*\/\s*([\d.]+%?)\s*\)$/i) and normalize it to a percent
as currently done, and modify stripAlpha() to accept unit-containing color
components (allow letters like "deg" and % in the middle capture) and also
detect legacy comma-separated alpha including percentage values so that both
extractAlpha() and stripAlpha() produce matching base-color and alpha values
used by replacement().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cc8d43fd-5274-44e0-8f10-6a888ee9c7ca
📒 Files selected for processing (1)
packages/tailwindcss/src/utils/replace-shadow-colors.ts
Comment on lines
+10
to
+23
| function extractAlpha(color: string): string | null { | ||
| // Modern rgba/hsla syntax with slash: rgba(0 0 0 / 0.12) or rgba(0 0 0 / 12%) | ||
| const slashAlphaMatch = color.match(/\/\s*([\d.]+)%?\s*\)/) | ||
| if (slashAlphaMatch) { | ||
| let alpha = slashAlphaMatch[1] | ||
| // If it's already a percentage, return it | ||
| if (slashAlphaMatch[0].includes('%')) { | ||
| return `${alpha}%` | ||
| } | ||
| // Convert decimal to percentage | ||
| const alphaNum = parseFloat(alpha) | ||
| if (!isNaN(alphaNum)) { | ||
| // Round to avoid floating point precision issues | ||
| return `${Math.round(alphaNum * 100)}%` |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "replace-shadow-colors.ts" | head -20Repository: tailwindlabs/tailwindcss
Length of output: 125
🏁 Script executed:
cat -n ./packages/tailwindcss/src/utils/replace-shadow-colors.tsRepository: tailwindlabs/tailwindcss
Length of output: 5588
🏁 Script executed:
# Test the actual regex patterns with concrete examples to fully verify
python3 << 'EOF'
import re
# Test case 1: hsl with deg units and slash alpha
test_color_1 = "hsl(180deg 50% 50% / 0.3)"
# Current extractAlpha slash pattern
extract_slash_current = r"/\s*([\d.]+)%?\s*\)"
match = re.search(extract_slash_current, test_color_1)
print(f"extractAlpha current pattern on '{test_color_1}': {bool(match)}")
if match:
print(f" Captured groups: {match.groups()}")
# Current stripAlpha slash pattern
strip_slash_current = r"^(rgba?|hsla?)\(([\d\s.%]+)\s*\/\s*[\d.]+%?\s*\)$"
match = re.search(strip_slash_current, test_color_1, re.IGNORECASE)
print(f"stripAlpha current pattern on '{test_color_1}': {bool(match)}")
# Test case 2: rgba with percentage alpha (comma form)
test_color_2 = "rgba(0, 0, 0, 12%)"
# Current extractAlpha comma pattern
extract_comma_current = r"^(?:rgba|hsla)\([^,]+,[^,]+,[^,]+,\s*([\d.]+)\s*\)$"
match = re.search(extract_comma_current, test_color_2, re.IGNORECASE)
print(f"\nextractAlpha current pattern on '{test_color_2}': {bool(match)}")
# Test case 3: Standard rgba with decimal alpha (comma form) - should work
test_color_3 = "rgba(0, 0, 0, 0.12)"
match = re.search(extract_comma_current, test_color_3, re.IGNORECASE)
print(f"extractAlpha current pattern on '{test_color_3}': {bool(match)}")
if match:
print(f" Captured groups: {match.groups()}")
# Test case 4: stripAlpha with percentage comma alpha
test_color_2 = "rgba(0, 0, 0, 12%)"
strip_comma_current = r"^(rgba?|hsla?)\(([\d\s.,%]+),\s*[\d.]+\s*\)$"
match = re.search(strip_comma_current, test_color_2, re.IGNORECASE)
print(f"stripAlpha current pattern on '{test_color_2}': {bool(match)}")
# Test case 5: stripAlpha with decimal comma alpha - should work
test_color_3 = "rgba(0, 0, 0, 0.12)"
match = re.search(strip_comma_current, test_color_3, re.IGNORECASE)
print(f"stripAlpha current pattern on '{test_color_3}': {bool(match)}")
if match:
print(f" Captured groups: {match.groups()}")
EOFRepository: tailwindlabs/tailwindcss
Length of output: 531
🏁 Script executed:
# Verify the proposed fix patterns work correctly
python3 << 'EOF'
import re
test_cases = [
("hsl(180deg 50% 50% / 0.3)", True, "modern hsl with deg and decimal alpha"),
("rgb(0 0 0 / 0.12)", True, "modern rgb with decimal alpha"),
("rgba(255 128 64 / 30%)", True, "modern rgba with percentage alpha"),
("hsla(180deg 50% 50% / 0.3)", True, "modern hsla with deg and decimal alpha"),
("rgba(0, 0, 0, 0.12)", True, "legacy rgba with decimal alpha"),
("rgba(0, 0, 0, 12%)", True, "legacy rgba with percentage alpha"),
("hsla(180, 50%, 50%, 0.3)", True, "legacy hsla with decimal alpha"),
("hsla(180, 50%, 50%, 30%)", True, "legacy hsla with percentage alpha"),
("rgb(0 0 0)", False, "no alpha"),
("currentcolor", False, "not a function"),
]
print("=" * 80)
print("PROPOSED extractAlpha PATTERNS")
print("=" * 80)
# Proposed slash pattern from diff
proposed_slash = r"^(rgba?|hsla?)\((.+)\s*\/\s*([\d.]+%?)\s*\)$"
print(f"\nSlash pattern: {proposed_slash}")
print("-" * 80)
for color, should_match, desc in test_cases:
if '/' not in color:
continue
match = re.match(proposed_slash, color, re.IGNORECASE)
status = "✓" if bool(match) == should_match else "✗"
print(f"{status} '{color}'")
print(f" Expected: {should_match}, Got: {bool(match)} ({desc})")
if match:
print(f" Groups: func={match.group(1)}, values={match.group(2)}, alpha={match.group(3)}")
# Proposed comma pattern from diff
proposed_comma = r"^(?:rgba|hsla)\(([^,]+,[^,]+,[^,]+),\s*([\d.]+%?)\s*\)$"
print(f"\n\nComma pattern: {proposed_comma}")
print("-" * 80)
for color, should_match, desc in test_cases:
if ',' not in color:
continue
match = re.match(proposed_comma, color, re.IGNORECASE)
status = "✓" if bool(match) == should_match else "✗"
print(f"{status} '{color}'")
print(f" Expected: {should_match}, Got: {bool(match)} ({desc})")
if match:
print(f" Groups: values={match.group(1)}, alpha={match.group(2)}")
print("\n" + "=" * 80)
print("PROPOSED stripAlpha PATTERNS")
print("=" * 80)
# Proposed stripAlpha slash pattern from diff
proposed_strip_slash = r"^(rgba?|hsla?)\((.+)\s*\/\s*[\d.]+%?\s*\)$"
print(f"\nSlash pattern: {proposed_strip_slash}")
print("-" * 80)
strip_test_cases = [
("hsl(180deg 50% 50% / 0.3)", "hsl(180deg 50% 50% )", "modern hsl with deg"),
("rgb(0 0 0 / 0.12)", "rgb(0 0 0)", "modern rgb"),
("rgba(0, 0, 0, 0.12)", None, "legacy - handled by comma pattern"),
]
for color, expected_result, desc in strip_test_cases:
if '/' not in color:
continue
match = re.match(proposed_strip_slash, color, re.IGNORECASE)
status = "✓" if bool(match) else "✗"
print(f"{status} '{color}' ({desc})")
if match:
func_type = match.group(1).lower().replace('a', '')
values = match.group(2).strip()
result = f"{func_type}({values})"
print(f" Result: '{result}'")
# Proposed stripAlpha comma pattern from diff
proposed_strip_comma = r"^(rgba|hsla)\(([^,]+,[^,]+,[^,]+),\s*[\d.]+%?\s*\)$"
print(f"\nComma pattern: {proposed_strip_comma}")
print("-" * 80)
comma_strip_test = [
("rgba(0, 0, 0, 0.12)", "rgb(0, 0, 0)"),
("rgba(0, 0, 0, 12%)", "rgb(0, 0, 0)"),
("hsla(180, 50%, 50%, 0.3)", "hsl(180, 50%, 50%)"),
]
for color, expected_result in comma_strip_test:
match = re.match(proposed_strip_comma, color, re.IGNORECASE)
status = "✓" if bool(match) else "✗"
print(f"{status} '{color}'")
if match:
func_type = match.group(1).lower().replace('a', '')
values = match.group(2).strip()
result = f"{func_type}({values})"
print(f" Result: '{result}'")
EOFRepository: tailwindlabs/tailwindcss
Length of output: 2425
Fix regex mismatch between extractAlpha() and stripAlpha() for consistent color/alpha handling.
Current code has misaligned patterns that create color parsing failures:
extractAlpha()matcheshsl(180deg 50% 50% / 0.3)butstripAlpha()rejects it (character class[\d\s.%]+excludes unit letters likedeg), leaving/ 0.3in the base color passed toreplacement().- Neither function handles percentage alphas in legacy comma form:
rgba(0, 0, 0, 12%)andhsla(..., 30%)are never extracted, so custom--tw-shadow-coloroverrides lose per-shadow opacity.
The proposed fix patterns are correct, but the diff contains a group index error: the slash pattern /^(rgba?|hsla?)\((.+)\s*\/\s*([\d.]+%?)\s*\)$/i captures alpha in group [3], not [2] as shown in the diff. Correct the assignment to slashAlphaMatch[3] and apply the corresponding fixes to stripAlpha() to handle both modern (slash) and legacy (comma) syntax consistently.
🤖 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 10 -
23, extractAlpha() is using the wrong capture index for the "slash" regex and
stripAlpha() has a mismatched pattern so modern hsl/rgba with unit tokens (like
"deg") or legacy percentage alphas (e.g. "rgba(..., 12%)" / "hsla(..., 30%)")
are not handled consistently; update extractAlpha() to pull the alpha from
slashAlphaMatch[3] (the third capture in
/^(rgba?|hsla?)\((.+)\s*\/\s*([\d.]+%?)\s*\)$/i) and normalize it to a percent
as currently done, and modify stripAlpha() to accept unit-containing color
components (allow letters like "deg" and % in the middle capture) and also
detect legacy comma-separated alpha including percentage values so that both
extractAlpha() and stripAlpha() produce matching base-color and alpha values
used by replacement().
Comment on lines
+28
to
+30
| // 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`n const commaAlphaMatch = color.match(/^(?:rgba|hsla)\([^,]+,[^,]+,[^,]+,\s*([\d.]+)\s*\)$/i) | ||
| if (commaAlphaMatch) { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/tailwindcss/src/utils/replace-shadow-colors.ts | sed -n '25,35p'Repository: tailwindlabs/tailwindcss
Length of output: 627
🏁 Script executed:
rg "commaAlphaMatch" packages/tailwindcss/src/utils/replace-shadow-colors.tsRepository: tailwindlabs/tailwindcss
Length of output: 299
🏁 Script executed:
cd packages/tailwindcss && [ -f tsconfig.json ] && cat tsconfig.json || echo "No tsconfig.json found"Repository: tailwindlabs/tailwindcss
Length of output: 109
Restore the commaAlphaMatch declaration to code.
The const commaAlphaMatch = ... declaration on line 29 is inside the // comment, making it unreachable. Line 30 references an undefined identifier and the file fails type-checking.
🐛 Proposed fix
// 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`n const commaAlphaMatch = color.match(/^(?:rgba|hsla)\([^,]+,[^,]+,[^,]+,\s*([\d.]+)\s*\)$/i)
+ // 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.
Suggested change
| // 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`n const commaAlphaMatch = color.match(/^(?:rgba|hsla)\([^,]+,[^,]+,[^,]+,\s*([\d.]+)\s*\)$/i) | |
| if (commaAlphaMatch) { | |
| // 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) { |
🤖 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 -
30, Restore the missing declaration for the regex match by re-adding the const
commaAlphaMatch =
color.match(/^(?:rgba|hsla)\([^,]+,[^,]+,[^,]+,\s*([\d.]+)\s*\)$/i) so the
subsequent if (commaAlphaMatch) branch in replace-shadow-colors.ts can access
the parsed alpha; locate where commaAlphaMatch is currently referenced and
either uncomment or reinsert that exact declaration (using the same regex)
immediately before the if (commaAlphaMatch) check to fix the undefined
identifier and restore type-checking.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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