-
Notifications
You must be signed in to change notification settings - Fork 0
feat(docs): add showcase pages demonstrating Shift CSS features #49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a Showcase section to the docs: a new ShowcaseLayout component, three showcase pages (dashboard, landing, article), navigation changes to open showcase links in a new tab with an indicator, and small CSS/token updates for text contrast and selector specificity. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@apps/docs/src/components/navigation/MobileMenu.astro`:
- Around line 54-67: When isShowcase is true the link visually indicates an
external/new-tab target but doesn't announce it to screen readers; update the
anchor (the element rendered where isShowcase is computed, the <a ...
data-mobile-link> block) to include an accessible cue by either adding an
aria-label that appends "opens in a new tab" to item.label for showcase links or
by referencing a visually-hidden text node via aria-describedby; ensure the
visual external-icon span remains aria-hidden and apply the same change to the
other occurrence (the similar anchor at the second block noted in the comment).
In `@apps/docs/src/components/navigation/SidebarGroup.astro`:
- Around line 19-31: For showcase links (where isShowcase is true) update the
anchor rendering to expose the "opens in new tab" affordance to assistive tech:
when isShowcase is true add either an aria-label like `${item.label} (opens in
new tab)` or append a visually-hidden span (e.g. class "sr-only") with the text
"opens in new tab" inside the <a>; keep the existing target and rel props and
the visible external-icon, and make the same change for the other showcase link
block (lines with the same pattern around the external-icon).
In `@apps/docs/src/pages/showcase/article.astro`:
- Around line 279-307: The TOC anchor elements with class "toc-item" currently
use href="#" and should instead link to real section IDs; add id attributes to
the corresponding article heading elements (e.g., "The Rise of Container
Queries", "OKLCH: Perceptually Uniform Color", etc.) using stable slugified
names (e.g., id="rise-of-container-queries") and update each <a
class="toc-item"> href to the matching fragment (e.g.,
href="#rise-of-container-queries"); ensure IDs are unique, match the visible
heading text, and update any active-state handling to rely on the fragment or
aria-current for keyboard/screen-reader navigation.
In `@apps/docs/src/pages/showcase/dashboard.astro`:
- Around line 15-53: The mobile menu toggle currently uses non-focusable
<label>s (see mobile-menu-btn and mobile-close-btn) with a hidden checkbox
(mobile-menu-toggle), making the controls inaccessible; either replace those
labels with real <button> elements that toggle the checkbox and maintain
aria-expanded, or keep the checkbox and add a small JS handler that selects
`#mobile-menu-toggle`, .mobile-menu-btn, .mobile-close-btn and
.mobile-menu-overlay, adds click listeners to set toggle.checked true/false,
listens for toggle change, and updates aria-expanded on the buttons accordingly
(ensure syncState runs on load).
- Around line 124-138: The Export and New Order buttons render icon-only at
small breakpoints because their visible labels are hidden with s-hide-on="sm";
add an accessible name to each button (e.g., set aria-label="Export" and
aria-label="New Order" or reference a visually-hidden text element with
aria-labelledby) so screen readers still announce the control; target the two
button elements with s-btn="outline" s-tooltip="Download report" and
s-btn="primary" to apply the aria attributes or add sr-only spans.
- Around line 406-439: The interactive quick-action tiles in the showcase
dashboard are plain <div>s (the four elements with attributes s-card
s-interactive s-flex class="s-py-8" s-tooltip and labels "New Invoice", "Add
Member", "Reports", "Settings") and must be converted to semantic,
keyboard-accessible controls; replace each interactive <div> with a <button> (or
<a> if it navigates) preserving all styling attributes (s-card, s-interactive,
s-flex, s-py-8) and move any click handlers to the new element, add an
accessible label via aria-label or ensure the visible <span
class="s-font-medium"> text is exposed, and keep the tooltip behavior by mapping
s-tooltip to an accessible attribute (aria-label/aria-describedby) so the
actions are operable via keyboard and screen readers.
🧹 Nitpick comments (1)
apps/docs/src/components/layout/ShowcaseLayout.astro (1)
21-27: Avoidis:inlinein Astro v5 inline scripts.Astro v5 recommends omitting
is:inlineso the script is processed with the default TypeScript pipeline.♻️ Suggested change
- <script is:inline> + <script>
Summary
Add three showcase pages demonstrating Shift CSS's capabilities in real-world scenarios:
/showcase/landing) - SaaS marketing page with hero, features, pricing, testimonials, and mobile-responsive navigation/showcase/dashboard) - Admin dashboard with stats cards, activity feed, and responsive layout/showcase/article) - Long-form content page with prose styling, TOC sidebar, and code blocksAlso includes:
ShowcaseLayout.astrocomponent using vanilla Shift CSS (no docs theme overrides)--s-text-tertiarycontrast for dark mode (core framework fix)Related Issues
N/A
Checklist
bun run lintpassesbun run buildsucceedsType of Change
Additional Notes
Files Changed
ShowcaseLayout.astro- Standalone layout with theme controlslanding.astro- Full marketing landing page with mobile menudashboard.astro- Admin dashboard with DiceBear avatarsarticle.astro- Article page with proper code block stylingsemantic.json- Fixed tertiary text dark mode contrast (neutral-500 → neutral-400)