Fix carousel dots + CircularProgress accent to follow platform accent#1497
Merged
Merged
Conversation
… [preview:none] Two shared components hardcoded the Flamingo brand color on state indicators that should track the platform-adaptive accent, so they rendered pink/cyan on every platform instead of the platform accent (yellow on openframe/openmsp, pink only on flamingo/flamingo-teaser): - MediaCarousel active pagination dot: bg-ods-flamingo-pink -> bg-ods-accent - CircularProgress accent variant: --ods-flamingo-cyan-base -> --ods-accent (siblings success/warning/error already use their semantic tokens) Both now consume --color-accent-primary / --ods-accent, matching the per-[data-app-type] overrides in ods-colors.css. Co-Authored-By: Claude Opus 4.8 <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughUpdated active media carousel dots and accent circular-progress rings to use the platform accent tokens, with corresponding comments revised. ChangesAccent token updates
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
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
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.
What
Two shared components hardcoded the Flamingo brand color on state indicators that should track the platform-adaptive accent token. As a result they rendered pink/cyan on every platform instead of the platform accent (yellow on openframe/openmsp, pink only on flamingo/flamingo-teaser).
MediaCarouselactive pagination dot:bg-ods-flamingo-pink→bg-ods-accentCircularProgressaccentvariant:--ods-flamingo-cyan-base→--ods-accent(its sibling variantssuccess/warning/erroralready use their semantic tokens;accentwas the odd one out)Comments documenting the old "always pink" intent were updated to say "platform accent".
Why
ods-flamingo-pink/ods-flamingo-cyanare theme-independent brand tokens. Themed controls and state indicators should consume--ods-accent/--color-accent-primary, whichods-colors.cssoverrides per[data-app-type]. The reported bug was the carousel controls always showing Flamingo pink regardless of platform; this fixes that class of issue at the shared-component level.Reviewer notes
bg-black/50+text-ods-text-on-dark) — untouched.--ods-accent, so the dot now matches them.who-am-i,openframe-features-section, andvendor-detail-content. Needs the usual lib release → hub pin bump to reach production.🤖 Generated with Claude Code
Summary by CodeRabbit