Update library landing pages and refresh shared site components - #1075
Open
Abeuty wants to merge 1 commit into
Open
Update library landing pages and refresh shared site components#1075Abeuty wants to merge 1 commit into
Abeuty wants to merge 1 commit into
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
tanstack-com | bda5417 | Commit Preview URL Branch Preview URL |
Jul 29 2026, 10:27 PM |
📝 WalkthroughWalkthroughThe PR refreshes shared design-system components, responsive navigation, maintainer and library views, homepage sections, branding previews, footer navigation, and application-starter styling. It also adds maintainer documentation routes, responsive media behavior, theme tokens, and accessibility-related motion handling. ChangesDesign system and component previews
Maintainer and library experiences
Navigation and shared controls
Homepage and landing presentation
Application and site chrome
Estimated code review effort: 5 (Critical) | ~90 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/components/MaintainerCard.tsx (2)
395-395: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
MaintainerCardPropsstill declareslibraryIdandhideLibraries, which are now ignored.Callers can keep passing them with no effect. Trim the type (or keep the props and honor them) so the contract matches the behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/MaintainerCard.tsx` at line 395, Update MaintainerCardProps and the MaintainerCard API to remove the unused libraryId and hideLibraries props, ensuring the declared contract matches the component’s current behavior; alternatively, implement their intended behavior if those props must remain supported.
190-199: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
slice(0, 2)picks social links by object key order.Which platforms surface on the card depends on the declaration order in
src/libraries/maintainers.tsper maintainer, so cards are inconsistent andwebsitecan silently disappear. Consider an explicit priority list (e.g.['twitter', 'bluesky', 'website']) instead of positional truncation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/MaintainerCard.tsx` around lines 190 - 199, Update the card branch in MaintainerCard’s socialEntries construction to select links using an explicit platform-priority list rather than Object.entries(...).slice(0, 2). Preserve the existing GitHub entry and ensure prioritized platforms such as twitter, bluesky, and website are selected consistently without being dropped due to object key order.src/components/ds/DsKit.tsx (1)
22-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClass concatenation in
DsDescriptionmakes caller overrides order-dependent.DsDescriptionjoins its role classes with the incomingclassNameinstead of merging them, so any caller utility that conflicts with a role default (margins, text size, color) coexists with it and resolution falls back to CSS source order.
src/components/ds/DsKit.tsx#L22-L28: replace the arrayjoin(' ')with the already-importedtwMerge(descriptionStyles[role], className).src/routes/ds.typography.tsx#L343-L345: after the merge fix,className="mt-2"will reliably override the role'smt-3/mt-1; no change needed here beyond confirming the intended spacing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ds/DsKit.tsx` around lines 22 - 28, Update DsDescription in src/components/ds/DsKit.tsx:22-28 to use the imported twMerge with descriptionStyles[role] and className instead of array joining, so caller utilities reliably override role defaults. In src/routes/ds.typography.tsx:343-345, make no code change; only confirm the existing mt-2 spacing override remains intended.src/components/ds/BrandAssets.tsx (1)
124-157: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider
loading="lazy"/ explicit dimensions on the preview<img>.The gallery renders a dozen-plus square previews; lazy loading and intrinsic sizing would avoid layout shift and unnecessary fetches on this catalog page.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ds/BrandAssets.tsx` around lines 124 - 157, Update the preview img in AssetCard to use lazy loading and explicit square dimensions matching the gallery layout, while preserving asset.preview, asset.alt, and asset.imgClass. Ensure the intrinsic sizing prevents layout shift and offscreen previews are not fetched unnecessarily.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/MaintainerCard.tsx`:
- Around line 397-399: The MaintainerCard wrapper div’s aria-label is
ineffective without an accessible role. Remove this redundant aria-label, or
assign an appropriate role such as group/listitem and connect it to the existing
maintainer-name-${maintainer.github} element via aria-labelledby.
- Around line 401-408: Update the GitHub profile anchor in MaintainerCard to
render maintainer.avatar when it is present, while retaining the gray background
as the fallback for missing avatars. Preserve the existing link, accessibility
attributes, and layout styling.
In `@src/routes/ds.typography.tsx`:
- Around line 343-345: Remove the hardcoded mt-2 class from the DsDescription
usage in the role-based typography rendering, allowing the role defaults (mt-3
for page and mt-1 for section) to apply without conflicting margin utilities.
In `@src/styles/app.css`:
- Around line 580-628: Add the missing light-mode declarations to .ds-mode-light
for every token overridden by html.dark: --color-icon-*,
--color-action-primary*, --color-status-*, --color-background-elevated,
--color-border-error/success, --color-text-accent, --color-input-bg-hover,
--color-menu-item-pressed, and --color-divider-*. Reuse the existing light-theme
token values so components such as ButtonMode and text-icon-muted remain fully
light-themed inside previews.
---
Nitpick comments:
In `@src/components/ds/BrandAssets.tsx`:
- Around line 124-157: Update the preview img in AssetCard to use lazy loading
and explicit square dimensions matching the gallery layout, while preserving
asset.preview, asset.alt, and asset.imgClass. Ensure the intrinsic sizing
prevents layout shift and offscreen previews are not fetched unnecessarily.
In `@src/components/ds/DsKit.tsx`:
- Around line 22-28: Update DsDescription in src/components/ds/DsKit.tsx:22-28
to use the imported twMerge with descriptionStyles[role] and className instead
of array joining, so caller utilities reliably override role defaults. In
src/routes/ds.typography.tsx:343-345, make no code change; only confirm the
existing mt-2 spacing override remains intended.
In `@src/components/MaintainerCard.tsx`:
- Line 395: Update MaintainerCardProps and the MaintainerCard API to remove the
unused libraryId and hideLibraries props, ensuring the declared contract matches
the component’s current behavior; alternatively, implement their intended
behavior if those props must remain supported.
- Around line 190-199: Update the card branch in MaintainerCard’s socialEntries
construction to select links using an explicit platform-priority list rather
than Object.entries(...).slice(0, 2). Preserve the existing GitHub entry and
ensure prioritized platforms such as twitter, bluesky, and website are selected
consistently without being dropped due to object key order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 81b49e90-b99f-4803-94ab-27611ecc13fb
⛔ Files ignored due to path filters (11)
public/favicon-dark.svgis excluded by!**/*.svgpublic/favicon-light.svgis excluded by!**/*.svgpublic/images/brand/social/mark-dark.svgis excluded by!**/*.svgpublic/images/brand/social/mark-dark@2x.pngis excluded by!**/*.pngpublic/images/brand/social/mark-light.svgis excluded by!**/*.svgpublic/images/brand/social/mark-light@2x.pngis excluded by!**/*.pngpublic/images/brand/social/stacked-dark.svgis excluded by!**/*.svgpublic/images/brand/social/stacked-dark@2x.pngis excluded by!**/*.pngpublic/images/brand/social/stacked-light.svgis excluded by!**/*.svgpublic/images/brand/social/stacked-light@2x.pngis excluded by!**/*.pngpublic/images/hero-palm-motion.mp4is excluded by!**/*.mp4
📒 Files selected for processing (48)
src/components/ApplicationStarter.tsxsrc/components/ButtonGroup.tsxsrc/components/CopyPageDropdown.tsxsrc/components/Footer.tsxsrc/components/LibrariesOverlay.tsxsrc/components/LibraryStatusBadge.tsxsrc/components/MaintainerCard.tsxsrc/components/MaintainersSection.tsxsrc/components/MegaMenuItem.tsxsrc/components/Navbar.tsxsrc/components/NavbarAuthControls.tsxsrc/components/NavbarCartButton.tsxsrc/components/OpenSourceStats.tsxsrc/components/PartnersSection.tsxsrc/components/PartnersSponsorsSection.tsxsrc/components/SearchButton.tsxsrc/components/ThemeToggle.tsxsrc/components/application-builder/useApplicationBuilder.tsxsrc/components/ds/BrandAssets.tsxsrc/components/ds/DsKit.tsxsrc/components/ds/ds-nav.tssrc/components/ds/ui/BlogPostCard.tsxsrc/components/ds/ui/StatsSection.tsxsrc/components/ds/ui/index.tsxsrc/components/home/HomeCommunitySection.tsxsrc/components/home/HomeNewsletterSection.tsxsrc/components/home/HomeSocialProofSection.tsxsrc/components/home/HomeStatsSection.tsxsrc/components/landing/DevtoolsLanding.tsxsrc/components/landing/HighlightLanding.tsxsrc/components/landing/LibraryLanding.tsxsrc/components/landing/MarkdownLanding.tsxsrc/components/stack/CategoryArticle.tsxsrc/contexts/LibrariesOverlayContext.tsxsrc/routeTree.gen.tssrc/routes/__root.tsxsrc/routes/ds.badges.tsxsrc/routes/ds.buttons.tsxsrc/routes/ds.cards.tsxsrc/routes/ds.logos.tsxsrc/routes/ds.maintainers.tsxsrc/routes/ds.typography.tsxsrc/routes/index.tsxsrc/routes/maintainers.tsxsrc/routes/paid-support.tsxsrc/styles/app.csssrc/ui/Badge.tsxsrc/ui/Tooltip.tsx
Comment on lines
397
to
399
| <div | ||
| className="group bg-white dark:bg-gray-900 rounded-lg overflow-hidden shadow-xs border border-gray-200 dark:border-gray-800" | ||
| className="group flex w-full max-w-[252px] flex-col items-center rounded-[26px] corner-squircle p-1.5 transition-colors duration-200 hover:bg-[#f2f2f2] focus-within:bg-[#f2f2f2] active:bg-[#e5e5e5] motion-reduce:transition-none dark:hover:bg-[#262626] dark:focus-within:bg-[#262626] dark:active:bg-[#404040]" | ||
| aria-label={`Maintainer card for ${maintainer.name}`} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
aria-label on a generic <div> isn't exposed to assistive tech.
Without a role, the label is dropped. Either drop it (the inner link already carries a descriptive label) or give the wrapper a role such as group/listitem — the existing id={maintainer-name-${maintainer.github}} on Line 412 could then serve as aria-labelledby.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/MaintainerCard.tsx` around lines 397 - 399, The MaintainerCard
wrapper div’s aria-label is ineffective without an accessible role. Remove this
redundant aria-label, or assign an appropriate role such as group/listitem and
connect it to the existing maintainer-name-${maintainer.github} element via
aria-labelledby.
Comment on lines
401
to
+408
| <a | ||
| href={`https://github.com/${maintainer.github}`} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| aria-label={`View ${maintainer.name}'s GitHub profile`} | ||
| className="relative h-64 overflow-hidden block" | ||
| className="block aspect-square w-full overflow-hidden rounded-[22px] corner-squircle bg-[#d9d9d9] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-focus" | ||
| tabIndex={0} | ||
| > | ||
| <img | ||
| alt={`Avatar of ${maintainer.name}`} | ||
| className="w-full h-full object-cover group-hover:scale-110 transition-transform duration-500" | ||
| src={maintainer.avatar} | ||
| loading="lazy" | ||
| decoding="async" | ||
| style={{ | ||
| aspectRatio: '1/1', | ||
| objectFit: 'cover', | ||
| }} | ||
| /> | ||
| <div className="absolute inset-0 bg-linear-to-t from-black/60 via-black/30 to-transparent opacity-0 group-hover:opacity-100 transition-opacity duration-300" /> | ||
| <div className="absolute inset-0 p-4 flex flex-col justify-end opacity-0 group-hover:opacity-100 transition-opacity duration-300"> | ||
| <div className="space-y-2"> | ||
| {maintainer.frameworkExpertise && | ||
| maintainer.frameworkExpertise.length > 0 && ( | ||
| <div className="flex flex-wrap gap-2 mb-2"> | ||
| {maintainer.frameworkExpertise.map((framework) => ( | ||
| <FrameworkChip key={framework} framework={framework} /> | ||
| ))} | ||
| </div> | ||
| )} | ||
| {maintainer.specialties && maintainer.specialties.length > 0 && ( | ||
| <div className="flex flex-wrap gap-2"> | ||
| {maintainer.specialties.map((specialty) => ( | ||
| <SpecialtyChip key={specialty} specialty={specialty} /> | ||
| ))} | ||
| </div> | ||
| )} | ||
| </div> | ||
| </div> | ||
| </a> | ||
| <div className="p-3 space-y-2"> | ||
| <div className="flex items-center justify-between"> | ||
| <span | ||
| className="text-base font-bold" | ||
| id={`maintainer-name-${maintainer.github}`} | ||
| > | ||
| {maintainer.name} | ||
| </span> | ||
| <div className="flex items-center gap-2"> | ||
| {libraryId && ( | ||
| <RoleBadge maintainer={maintainer} libraryId={libraryId} /> | ||
| )} | ||
| </div> | ||
| </div> | ||
| {!hideLibraries && !libraryId && libraries.length > 0 && ( | ||
| <div className="flex flex-wrap gap-1.5 pt-1"> | ||
| {libraries | ||
| .slice(0, showAllLibraries ? undefined : 2) | ||
| .map((library) => ( | ||
| <LibraryBadge key={library.id} library={library} /> | ||
| ))} | ||
| {!showAllLibraries && libraries.length > 2 && ( | ||
| <button | ||
| onClick={(e) => { | ||
| e.preventDefault() | ||
| e.stopPropagation() | ||
| setShowAllLibraries(true) | ||
| }} | ||
| className="inline-flex items-center px-2 py-0.5 rounded-full text-xs font-medium text-gray-500 dark:text-gray-400 hover:text-gray-700 dark:hover:text-gray-200 transition-colors focus:outline-none focus:ring-2 focus:ring-blue-400" | ||
| aria-label={`Show ${libraries.length - 2} more libraries`} | ||
| tabIndex={0} | ||
| type="button" | ||
| > | ||
| +{libraries.length - 2} more | ||
| </button> | ||
| )} | ||
| </div> | ||
| )} | ||
| <MaintainerSocialLinks maintainer={maintainer} /> | ||
| /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The avatar is never rendered — every card shows the bare gray placeholder.
The anchor has no children and maintainer.avatar is no longer read anywhere in this component, so bg-[#d9d9d9] is the permanent visual for real maintainers on /maintainers, /paid-support, MaintainersSection, and HomeCommunitySection. If the placeholder is intentional for the DS preview only, the image should still render when maintainer.avatar is present.
🐛 Proposed fix
className="block aspect-square w-full overflow-hidden rounded-[22px] corner-squircle bg-[`#d9d9d9`] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-focus"
tabIndex={0}
- />
+ >
+ {maintainer.avatar ? (
+ <img
+ src={maintainer.avatar}
+ alt=""
+ loading="lazy"
+ className="h-full w-full object-cover"
+ />
+ ) : null}
+ </a>📝 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
| <a | |
| href={`https://github.com/${maintainer.github}`} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| aria-label={`View ${maintainer.name}'s GitHub profile`} | |
| className="relative h-64 overflow-hidden block" | |
| className="block aspect-square w-full overflow-hidden rounded-[22px] corner-squircle bg-[#d9d9d9] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-focus" | |
| tabIndex={0} | |
| > | |
| <img | |
| alt={`Avatar of ${maintainer.name}`} | |
| className="w-full h-full object-cover group-hover:scale-110 transition-transform duration-500" | |
| src={maintainer.avatar} | |
| loading="lazy" | |
| decoding="async" | |
| style={{ | |
| aspectRatio: '1/1', | |
| objectFit: 'cover', | |
| }} | |
| /> | |
| <div className="absolute inset-0 bg-linear-to-t from-black/60 via-black/30 to-transparent opacity-0 group-hover:opacity-100 transition-opacity duration-300" /> | |
| <div className="absolute inset-0 p-4 flex flex-col justify-end opacity-0 group-hover:opacity-100 transition-opacity duration-300"> | |
| <div className="space-y-2"> | |
| {maintainer.frameworkExpertise && | |
| maintainer.frameworkExpertise.length > 0 && ( | |
| <div className="flex flex-wrap gap-2 mb-2"> | |
| {maintainer.frameworkExpertise.map((framework) => ( | |
| <FrameworkChip key={framework} framework={framework} /> | |
| ))} | |
| </div> | |
| )} | |
| {maintainer.specialties && maintainer.specialties.length > 0 && ( | |
| <div className="flex flex-wrap gap-2"> | |
| {maintainer.specialties.map((specialty) => ( | |
| <SpecialtyChip key={specialty} specialty={specialty} /> | |
| ))} | |
| </div> | |
| )} | |
| </div> | |
| </div> | |
| </a> | |
| <div className="p-3 space-y-2"> | |
| <div className="flex items-center justify-between"> | |
| <span | |
| className="text-base font-bold" | |
| id={`maintainer-name-${maintainer.github}`} | |
| > | |
| {maintainer.name} | |
| </span> | |
| <div className="flex items-center gap-2"> | |
| {libraryId && ( | |
| <RoleBadge maintainer={maintainer} libraryId={libraryId} /> | |
| )} | |
| </div> | |
| </div> | |
| {!hideLibraries && !libraryId && libraries.length > 0 && ( | |
| <div className="flex flex-wrap gap-1.5 pt-1"> | |
| {libraries | |
| .slice(0, showAllLibraries ? undefined : 2) | |
| .map((library) => ( | |
| <LibraryBadge key={library.id} library={library} /> | |
| ))} | |
| {!showAllLibraries && libraries.length > 2 && ( | |
| <button | |
| onClick={(e) => { | |
| e.preventDefault() | |
| e.stopPropagation() | |
| setShowAllLibraries(true) | |
| }} | |
| className="inline-flex items-center px-2 py-0.5 rounded-full text-xs font-medium text-gray-500 dark:text-gray-400 hover:text-gray-700 dark:hover:text-gray-200 transition-colors focus:outline-none focus:ring-2 focus:ring-blue-400" | |
| aria-label={`Show ${libraries.length - 2} more libraries`} | |
| tabIndex={0} | |
| type="button" | |
| > | |
| +{libraries.length - 2} more | |
| </button> | |
| )} | |
| </div> | |
| )} | |
| <MaintainerSocialLinks maintainer={maintainer} /> | |
| /> | |
| <a | |
| href={`https://github.com/${maintainer.github}`} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| aria-label={`View ${maintainer.name}'s GitHub profile`} | |
| className="block aspect-square w-full overflow-hidden rounded-[22px] corner-squircle bg-[`#d9d9d9`] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-focus" | |
| tabIndex={0} | |
| > | |
| {maintainer.avatar ? ( | |
| <img | |
| src={maintainer.avatar} | |
| alt="" | |
| loading="lazy" | |
| className="h-full w-full object-cover" | |
| /> | |
| ) : null} | |
| </a> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/MaintainerCard.tsx` around lines 401 - 408, Update the GitHub
profile anchor in MaintainerCard to render maintainer.avatar when it is present,
while retaining the gray background as the fallback for missing avatars.
Preserve the existing link, accessibility attributes, and layout styling.
Comment on lines
+343
to
+345
| <DsDescription role={role} className="mt-2"> | ||
| {children} | ||
| </DsDescription> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
className="mt-2" collides with the role's own top margin.
DsDescription concatenates classes rather than merging them, so mt-2 plus the role default (mt-3 for page, mt-1 for section) both land in the class list and the winner depends on CSS order. Root cause and fix are noted on src/components/ds/DsKit.tsx.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/ds.typography.tsx` around lines 343 - 345, Remove the hardcoded
mt-2 class from the DsDescription usage in the role-based typography rendering,
allowing the role defaults (mt-3 for page and mt-1 for section) to apply without
conflicting margin utilities.
Comment on lines
+580
to
+628
| .ds-mode-light { | ||
| color-scheme: light; | ||
| --color-text-primary: var(--color-ds-neutral-500); | ||
| --color-text-secondary: var(--color-ds-neutral-400); | ||
| --color-text-muted: var(--color-ds-neutral-300); | ||
| --color-text-inverse: #ffffff; | ||
| --color-background-default: #ffffff; | ||
| --color-background-surface: #ffffff; | ||
| --color-background-subtle: #fafafa; | ||
| --color-background-inverse: var(--color-ds-neutral-500); | ||
| --color-border-default: var(--color-ds-neutral-200); | ||
| --color-border-subtle: var(--color-ds-neutral-100); | ||
| --color-border-strong: var(--color-ds-neutral-400); | ||
| --color-border-focus: var(--color-ds-blue-400); | ||
| --color-action-secondary: var(--color-ds-neutral-100); | ||
| --color-action-secondary-hover: var(--color-ds-neutral-200); | ||
| --color-category-framework: var(--color-ds-green-400); | ||
| --color-category-data: var(--color-ds-terracotta-400); | ||
| --color-category-ui: var(--color-ds-blue-400); | ||
| --color-category-performance: var(--color-ds-amber-400); | ||
| --color-category-tooling: var(--color-ds-neutral-400); | ||
| --color-surface-state-hover: #1111110f; | ||
| --color-surface-state-pressed: #1111111f; | ||
| } | ||
|
|
||
| .ds-mode-dark { | ||
| color-scheme: dark; | ||
| --color-text-primary: #ffffff; | ||
| --color-text-secondary: #aea691; | ||
| --color-text-muted: #756c5b; | ||
| --color-text-inverse: #111111; | ||
| --color-background-default: #111111; | ||
| --color-background-surface: #1f1f1f; | ||
| --color-background-subtle: #1b1b1b; | ||
| --color-background-inverse: #ffffff; | ||
| --color-border-default: #2d2d2d; | ||
| --color-border-subtle: #232323; | ||
| --color-border-strong: #aea691; | ||
| --color-border-focus: #61adbf; | ||
| --color-action-secondary: #2b2b2b; | ||
| --color-action-secondary-hover: #3a3a3a; | ||
| --color-category-framework: var(--color-ds-green-300); | ||
| --color-category-data: var(--color-ds-terracotta-300); | ||
| --color-category-ui: var(--color-ds-blue-300); | ||
| --color-category-performance: var(--color-ds-amber-300); | ||
| --color-category-tooling: var(--color-ds-neutral-200); | ||
| --color-surface-state-hover: #ffffff1f; | ||
| --color-surface-state-pressed: #ffffff29; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
.ds-mode-light doesn't mirror every token html.dark overrides, so dark values bleed into light specimens.
html.dark overrides --color-icon-*, --color-action-primary*, --color-status-*, --color-background-elevated, --color-border-error/success, --color-text-accent, --color-input-bg-hover, --color-menu-item-pressed, and --color-divider-*; .ds-mode-light re-declares only a subset. When the site is in dark mode, a .ds-mode-light preview (e.g. ButtonMode mode="light" in src/routes/ds.buttons.tsx, or icon links using text-icon-muted) still resolves those to the dark values on a white surface. Worth adding the remaining overrides so previews are truly self-contained.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/styles/app.css` around lines 580 - 628, Add the missing light-mode
declarations to .ds-mode-light for every token overridden by html.dark:
--color-icon-*, --color-action-primary*, --color-status-*,
--color-background-elevated, --color-border-error/success, --color-text-accent,
--color-input-bg-hover, --color-menu-item-pressed, and --color-divider-*. Reuse
the existing light-theme token values so components such as ButtonMode and
text-icon-muted remain fully light-themed inside previews.
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
Testing
Not run (not requested)
Summary by CodeRabbit
New Features
Bug Fixes
Style