fix: update explorer subtitle text color for theme support#1281
fix: update explorer subtitle text color for theme support#1281VaiTon merged 1 commit intoopenfoodfacts:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe Explorer page subtitle styling was updated by changing its CSS class from Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/routes/explore/+page.svelte (1)
22-22: Fix for contrast is correct, but inconsistent with subtitle styling elsewhere in the codebase.The change from
text-secondary-contenttotext-primarycorrectly addresses issue#1280.text-secondary-contentwas semantically incorrect for text on abg-base-100background, and the theme configuration confirms excellent contrast in both light and dark modes (dark brown#201a17on light cream#faf7f5, and vice versa).However, the explore page's subtitle styling differs from the pattern used consistently across other pages:
- Landing page (
+page.svelte): usestext-base-content/80- Folksonomy page: uses
text-base font-normal opacity-70- Preferences form: uses
text-base-content/70For consistency with the existing design system, consider aligning the explore subtitle with this established muted-text pattern:
Suggested alignment with codebase pattern
- <p class="text-primary mb-6 max-w-2xl text-center text-lg"> + <p class="text-base-content/80 mb-6 max-w-2xl text-center text-lg"> {$_('explore.subtitle')} </p>This maintains visual hierarchy (heading in primary, subtitle muted) and aligns with the rest of the codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/explore/`+page.svelte at line 22, Replace the explore page subtitle's class on the <p class="text-primary mb-6 max-w-2xl text-center text-lg"> element: swap text-primary for the project’s muted-text utility (e.g., text-base-content/80 to match the landing page pattern or text-base-content/70 if you prefer the slightly stronger muted tone used elsewhere) so the subtitle follows the established design system while keeping the heading as text-primary; update the class string accordingly and verify visual contrast in both themes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/routes/explore/`+page.svelte:
- Line 22: Replace the explore page subtitle's class on the <p
class="text-primary mb-6 max-w-2xl text-center text-lg"> element: swap
text-primary for the project’s muted-text utility (e.g., text-base-content/80 to
match the landing page pattern or text-base-content/70 if you prefer the
slightly stronger muted tone used elsewhere) so the subtitle follows the
established design system while keeping the heading as text-primary; update the
class string accordingly and verify visual contrast in both themes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0aa16914-3ed9-4795-8ccc-0ff5bda41f9d
📒 Files selected for processing (1)
src/routes/explore/+page.svelte
|
Thank you! |
Summary
This PR addresses issue #1280.
What I changed
text-primary.Screenshots
fixes: #1280.
Summary by CodeRabbit