feat(skeleton): use new skeleton gradient color tokens#8084
feat(skeleton): use new skeleton gradient color tokens#8084tarunvashishth wants to merge 2 commits intopatternfly:mainfrom
Conversation
WalkthroughReplaced Skeleton component background and its ::after linear-gradient color stops to use loading-skeleton color tokens (including inside the prefers-reduced-motion media query) and changed the ::after gradient to a multi-stop gradient with transparent bands. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
Preview: https://pf-pr-8084.surge.sh A11y report: https://pf-pr-8084-a11y.surge.sh |
|
@tarunvashishth thanks for the PR! Looking at the PR preview (https://pf-pr-8084.surge.sh/components/skeleton), seems like the gradient isn't working properly somewhere, can you look into that and push a fix? For reference, the gradient should look similar to how it looks now. Here's what it looks like currently - https://www.patternfly.org/components/skeleton/html |
|
Sure, I will look into it. |
|
Thanks for the contribution @tarunvashishth ! I also noticed that the background of the skeleton component itself (not the :after) should probably also get the default skeleton color. |
![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
🤖 Fix all issues with AI agents
In `@src/patternfly/components/Skeleton/skeleton.scss`:
- Around line 27-29: Remove the redundant custom property overrides for the
linear-gradient color stops in skeleton.scss—specifically the variables
--#{$skeleton}--after--LinearGradientColorStop1,
--#{$skeleton}--after--LinearGradientColorStop2, and
--#{$skeleton}--after--LinearGradientColorStop3—because they duplicate the
defaults already declared earlier; keep only the differing properties
(TranslateX and AnimationName) in the reduced-motion vs no-preference blocks so
the color stops inherit the original defaults.
- Around line 16-18: Replace the two uses of the bare transparent color in the
skeleton gradient stops with the skeleton background color variable so the
shimmer doesn't interpolate through black: update
--#{$skeleton}--after--LinearGradientColorStop1 and
--#{$skeleton}--after--LinearGradientColorStop3 (and the identical pair inside
the reduced-motion media query) to use the subtle skeleton background var (the
same var used for the base background) instead of transparent so the gradient
blends smoothly.
| --#{$skeleton}--after--LinearGradientColorStop1: transparent; | ||
| --#{$skeleton}--after--LinearGradientColorStop2: var(--pf-t--global--background--color--loading--skeleton--default); | ||
| --#{$skeleton}--after--LinearGradientColorStop3: transparent; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
CSS gradient transparent keyword interpolation dark band artifacts
💡 Result:
In CSS, the keyword transparent is not “transparent version of the previous color”; it’s effectively transparent black (rgba(0,0,0,0)). (drafts.csswg.org)
So a gradient like:
background: linear-gradient(to right, `#ff0000`, transparent);interpolates from red → (black, alpha 0), which can create a dark/dirty band (especially noticeable mid‑gradient and when composited over light backgrounds). (drafts.csswg.org)
Fixes
- Fade alpha to 0 while keeping the same RGB
background: linear-gradient(to right,
rgb(255 0 0 / 1),
rgb(255 0 0 / 0)
);- Choose an interpolation color space explicitly (can reduce “muddy” transitions depending on colors)
background: linear-gradient(in srgb-linear, `#ff0000`, rgb(255 0 0 / 0));CSS gradients support the in <color-interpolation-method> syntax. (developer.mozilla.org)
If you share a minimal snippet (your two endpoint colors + background it’s drawn over + browser), I can suggest the best interpolation space and stops to eliminate the artifact.
Citations:
- 1: https://drafts.csswg.org/css-color-4/?utm_source=openai
- 2: https://drafts.csswg.org/css-color-4/?utm_source=openai
- 3: https://developer.mozilla.org/en-US/docs/Web/CSS/linear-gradient?utm_source=openai
🏁 Script executed:
# First, let's examine the skeleton.scss file to understand the context
find . -name "skeleton.scss" -type fRepository: patternfly/patternfly
Length of output: 115
🏁 Script executed:
# Now let's read the skeleton.scss file to see the actual context
cat -n ./src/patternfly/components/Skeleton/skeleton.scss | head -40Repository: patternfly/patternfly
Length of output: 2295
🏁 Script executed:
# Search for the subtle skeleton color token definition
rg "skeleton.*subtle" --max-count=20Repository: patternfly/patternfly
Length of output: 530
🏁 Script executed:
# Look for other skeleton color definitions
rg "background-color--loading--skeleton" --max-count=20Repository: patternfly/patternfly
Length of output: 47
transparent causes dark-band artifacts in the gradient shimmer.
Using the bare transparent keyword (which is rgba(0, 0, 0, 0)) makes the browser interpolate through semi-transparent black between the edge stops and the skeleton color in the middle. This is the gradient malfunction reported in the PR preview.
Use the subtle skeleton color (already defined as the base background on line 4) for stops 1 and 3 instead, so the shimmer blends smoothly. This applies to both instances: lines 16–18 and lines 27–29 (in the reduced-motion media query).
Proposed fix
- --#{$skeleton}--after--LinearGradientColorStop1: transparent;
+ --#{$skeleton}--after--LinearGradientColorStop1: var(--pf-t--global--background--color--loading--skeleton--subtle);
--#{$skeleton}--after--LinearGradientColorStop2: var(--pf-t--global--background--color--loading--skeleton--default);
- --#{$skeleton}--after--LinearGradientColorStop3: transparent;
+ --#{$skeleton}--after--LinearGradientColorStop3: var(--pf-t--global--background--color--loading--skeleton--subtle);📝 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.
| --#{$skeleton}--after--LinearGradientColorStop1: transparent; | |
| --#{$skeleton}--after--LinearGradientColorStop2: var(--pf-t--global--background--color--loading--skeleton--default); | |
| --#{$skeleton}--after--LinearGradientColorStop3: transparent; | |
| --#{$skeleton}--after--LinearGradientColorStop1: var(--pf-t--global--background--color--loading--skeleton--subtle); | |
| --#{$skeleton}--after--LinearGradientColorStop2: var(--pf-t--global--background--color--loading--skeleton--default); | |
| --#{$skeleton}--after--LinearGradientColorStop3: var(--pf-t--global--background--color--loading--skeleton--subtle); |
🧰 Tools
🪛 Stylelint (17.2.0)
[error] 17-17: Expected custom property name "--pf-t--global--background--color--loading--skeleton--default" to be kebab-case (custom-property-pattern)
(custom-property-pattern)
🤖 Prompt for AI Agents
In `@src/patternfly/components/Skeleton/skeleton.scss` around lines 16 - 18,
Replace the two uses of the bare transparent color in the skeleton gradient
stops with the skeleton background color variable so the shimmer doesn't
interpolate through black: update
--#{$skeleton}--after--LinearGradientColorStop1 and
--#{$skeleton}--after--LinearGradientColorStop3 (and the identical pair inside
the reduced-motion media query) to use the subtle skeleton background var (the
same var used for the base background) instead of transparent so the gradient
blends smoothly.
| --#{$skeleton}--after--LinearGradientColorStop1: transparent; | ||
| --#{$skeleton}--after--LinearGradientColorStop2: var(--pf-t--global--background--color--loading--skeleton--default); | ||
| --#{$skeleton}--after--LinearGradientColorStop3: transparent; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant overrides — these are identical to the defaults on Lines 16-18.
The color stop values here (transparent, skeleton--default, transparent) are the same as the defaults declared above. Only TranslateX and AnimationName differ between reduced-motion and no-preference. These three lines can be removed to reduce noise.
Proposed cleanup
`@media` screen and (prefers-reduced-motion: no-preference) {
- --#{$skeleton}--after--LinearGradientColorStop1: transparent;
- --#{$skeleton}--after--LinearGradientColorStop2: var(--pf-t--global--background--color--loading--skeleton--default);
- --#{$skeleton}--after--LinearGradientColorStop3: transparent;
--#{$skeleton}--after--TranslateX: -100%;
--#{$skeleton}--after--AnimationName: #{$skeleton}-loading;
}Note: If the color stop values are updated per the previous comment (replacing transparent with the subtle token), and you still want both paths to share the same stops, this redundancy argument still holds.
📝 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.
| --#{$skeleton}--after--LinearGradientColorStop1: transparent; | |
| --#{$skeleton}--after--LinearGradientColorStop2: var(--pf-t--global--background--color--loading--skeleton--default); | |
| --#{$skeleton}--after--LinearGradientColorStop3: transparent; | |
| `@media` screen and (prefers-reduced-motion: no-preference) { | |
| --#{$skeleton}--after--TranslateX: -100%; | |
| --#{$skeleton}--after--AnimationName: #{$skeleton}-loading; | |
| } |
🧰 Tools
🪛 Stylelint (17.2.0)
[error] 28-28: Expected custom property name "--pf-t--global--background--color--loading--skeleton--default" to be kebab-case (custom-property-pattern)
(custom-property-pattern)
[error] 27-27: Unexpected invalid position declaration (no-invalid-position-declaration)
(no-invalid-position-declaration)
[error] 28-28: Unexpected invalid position declaration (no-invalid-position-declaration)
(no-invalid-position-declaration)
[error] 29-29: Unexpected invalid position declaration (no-invalid-position-declaration)
(no-invalid-position-declaration)
🤖 Prompt for AI Agents
In `@src/patternfly/components/Skeleton/skeleton.scss` around lines 27 - 29,
Remove the redundant custom property overrides for the linear-gradient color
stops in skeleton.scss—specifically the variables
--#{$skeleton}--after--LinearGradientColorStop1,
--#{$skeleton}--after--LinearGradientColorStop2, and
--#{$skeleton}--after--LinearGradientColorStop3—because they duplicate the
defaults already declared earlier; keep only the differing properties
(TranslateX and AnimationName) in the reduced-motion vs no-preference blocks so
the color stops inherit the original defaults.
tarunvashishth
force-pushed
the
feat/skeleton-gradient-tokens
branch
from
37af2e3 to
1861ee9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/patternfly/components/Skeleton/skeleton.scss`:
- Around line 16-18: The shimmer gradient token roles are inverted: swap the
subtle/default tokens so the middle highlight is more prominent. Update the CSS
custom properties --#{$skeleton}--after--LinearGradientColorStop1 and
--#{$skeleton}--after--LinearGradientColorStop3 to use the base token
(var(--pf-t--global--background--color--loading--skeleton--default)) for the
edges, and set --#{$skeleton}--after--LinearGradientColorStop2 to the highlight
token (var(--pf-t--global--background--color--loading--skeleton--subtle) ->
change to var(--pf-t--global--background--color--loading--skeleton--default));
apply the same swap to the corresponding variables on the other block(s)
referenced (the ColorStop entries on lines 4,16-18,27-29 in this diff) so center
uses default (more opaque) and edges use subtle (less opaque).
| --#{$skeleton}--after--LinearGradientColorStop1: var(--pf-t--global--background--color--loading--skeleton--default); | ||
| --#{$skeleton}--after--LinearGradientColorStop2: var(--pf-t--global--background--color--loading--skeleton--subtle); | ||
| --#{$skeleton}--after--LinearGradientColorStop3: var(--pf-t--global--background--color--loading--skeleton--default); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the skeleton token definitions
rg -n "background--color--loading--skeleton" --type scss --type css -C 3Repository: patternfly/patternfly
Length of output: 93
🏁 Script executed:
# Also search in broader patterns for token definitions
rg "skeleton.*default|skeleton.*subtle" -t scss -t css -t json --max-count 20 -C 2Repository: patternfly/patternfly
Length of output: 93
🏁 Script executed:
# View the skeleton.scss file to see full context around lines 16-18
fd "skeleton.scss" -t f -x wc -l {}Repository: patternfly/patternfly
Length of output: 119
🏁 Script executed:
# View the skeleton.scss file with context
cat -n src/patternfly/components/Skeleton/skeleton.scss | head -50Repository: patternfly/patternfly
Length of output: 3422
🏁 Script executed:
# Search for skeleton loading token definitions
rg "background.*color.*loading.*skeleton" -C 3Repository: patternfly/patternfly
Length of output: 5096
🏁 Script executed:
# Search for token definitions or CSS variable assignments related to skeleton
rg "skeleton.*default|skeleton.*subtle" -C 2Repository: patternfly/patternfly
Length of output: 5062
🏁 Script executed:
# Verify the semantic context - check if there are any comments or documentation explaining token usage
rg "default|subtle" src/patternfly/base/tokens/tokens-default.scss -B 2 -A 2 | head -30Repository: patternfly/patternfly
Length of output: 2250
🏁 Script executed:
# Check if there are any design guidelines or comments about skeleton loading
rg "loading|shimmer" src/patternfly/components/Skeleton/ -C 2Repository: patternfly/patternfly
Length of output: 3231
Swap token roles: subtle for the base background, default for the shimmer highlight.
The token definitions confirm the semantic values: skeleton--default is 25% opacity (more prominent), while skeleton--subtle is 10% opacity (less prominent). In a shimmer loading effect, the highlight at ColorStop2 (center, 50%) should be more prominent than the base edges, but currently uses the subtle token. This is inverted.
Apply the fix across lines 4, 16-18, and 27-29:
- Line 4: Change
BackgroundColorfromdefaulttosubtle - Lines 16, 18, 27, 29: Keep ColorStop1/3 (edges) as
default - Lines 17, 28: Change ColorStop2 (center highlight) from
subtletodefault
Alternatively, swap all occurrences: edges→subtle, highlight→default.
🤖 Prompt for AI Agents
In `@src/patternfly/components/Skeleton/skeleton.scss` around lines 16 - 18, The
shimmer gradient token roles are inverted: swap the subtle/default tokens so the
middle highlight is more prominent. Update the CSS custom properties
--#{$skeleton}--after--LinearGradientColorStop1 and
--#{$skeleton}--after--LinearGradientColorStop3 to use the base token
(var(--pf-t--global--background--color--loading--skeleton--default)) for the
edges, and set --#{$skeleton}--after--LinearGradientColorStop2 to the highlight
token (var(--pf-t--global--background--color--loading--skeleton--subtle) ->
change to var(--pf-t--global--background--color--loading--skeleton--default));
apply the same swap to the corresponding variables on the other block(s)
referenced (the ColorStop entries on lines 4,16-18,27-29 in this diff) so center
uses default (more opaque) and edges use subtle (less opaque).
1861ee9 to
5f9970b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/patternfly/components/Skeleton/skeleton.scss`:
- Around line 16-18: The two custom properties
--#{$skeleton}--after--LinearGradientColorStop1 and
--#{$skeleton}--after--LinearGradientColorStop3 are declared but never used;
update the linear-gradient definition (the gradient referenced around lines
92–99) to use those variables instead of hardcoded transparent stops so the
gradient reads the declared color stops (e.g., replace bare "transparent" stops
with var(--#{$skeleton}--after--LinearGradientColorStop1) and
var(--#{$skeleton}--after--LinearGradientColorStop3)), which removes the dead
variables and prevents the black-interpolation artifact.
| --#{$skeleton}--after--LinearGradientColorStop1: var(--pf-t--global--background--color--loading--skeleton--subtle); | ||
| --#{$skeleton}--after--LinearGradientColorStop2: var(--pf-t--global--background--color--loading--skeleton--default); | ||
| --#{$skeleton}--after--LinearGradientColorStop3: var(--pf-t--global--background--color--loading--skeleton--subtle); |
There was a problem hiding this comment.
ColorStop1 and ColorStop3 are declared but never consumed.
The rewritten gradient (lines 92–99) only references ColorStop2; the transparent stops are hardcoded. This makes the ColorStop1 and ColorStop3 custom-property declarations dead code. Either:
- Remove the unused variables entirely, or
- Use them in the gradient instead of the bare
transparentkeyword — which also fixes the dark-band interpolation artifact reported in the preview (baretransparentisrgba(0,0,0,0)and interpolates through black).
Option 2 is the better path because it solves both problems at once.
Proposed fix (option 2): use the declared color-stop variables in the gradient
&::after {
`@include` pf-v6-mirror-inline-on-rtl;
position: absolute;
inset: 0;
display: block;
content: "";
background: linear-gradient(
var(--#{$skeleton}--after--LinearGradientAngle),
- transparent 0%,
- transparent 30%,
+ var(--#{$skeleton}--after--LinearGradientColorStop1) 0%,
+ var(--#{$skeleton}--after--LinearGradientColorStop1) 30%,
var(--#{$skeleton}--after--LinearGradientColorStop2) 50%,
- transparent 70%,
- transparent 100%
+ var(--#{$skeleton}--after--LinearGradientColorStop3) 70%,
+ var(--#{$skeleton}--after--LinearGradientColorStop3) 100%
);Also applies to: 92-99
🤖 Prompt for AI Agents
In `@src/patternfly/components/Skeleton/skeleton.scss` around lines 16 - 18, The
two custom properties --#{$skeleton}--after--LinearGradientColorStop1 and
--#{$skeleton}--after--LinearGradientColorStop3 are declared but never used;
update the linear-gradient definition (the gradient referenced around lines
92–99) to use those variables instead of hardcoded transparent stops so the
gradient reads the declared color stops (e.g., replace bare "transparent" stops
with var(--#{$skeleton}--after--LinearGradientColorStop1) and
var(--#{$skeleton}--after--LinearGradientColorStop3)), which removes the dead
variables and prevents the black-interpolation artifact.
4 participants
Fixes #7514
Updates the Skeleton component to use the new skeleton gradient color design tokens.
Summary by CodeRabbit