Skip to content

dox(spinners): fixes page hierarchy of components - #1121

Merged
tannerlinsley merged 3 commits into
TanStack:mainfrom
harry-whorlow:spinner-docs
Oct 20, 2026
Merged

tannerlinsley merged 3 commits into
TanStack:mainfrom
harry-whorlow:spinner-docs

Conversation

@harry-whorlow

@harry-whorlow harry-whorlow commented Aug 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Documentation
    • Updated the spinner page title to “Spinners.”
    • Added documentation for both Spinner and Headbanger indicators.
    • Included linked source references.
    • Improved the structure and presentation of indicator details.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9ca874e7-63c3-4975-b44d-47fd409080cb
📥 Commits

Reviewing files that changed from the base of the PR and between ec96b3a and cf09cef.

📒 Files selected for processing: 1
  • src/routes/ds.spinner.tsx
🚧 Files skipped from review as they are similar to previous changes: 1
  • src/routes/ds.spinner.tsx

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

📝 Walkthrough

The spinner page now uses the title “Spinners.” It documents the Spinner and Headbanger indicators in separate sections with formatted descriptions and source links.

Changes: Spinner page documentation

Layer / File(s) Summary
Indicator documentation
src/routes/ds.spinner.tsx
The page metadata and Spinner section are updated. The Headbanger description now uses formatted JSX to document its animation, color, sizing, reduced-motion behavior, and source link.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Suggested reviewers: abeuty

Merge Risk: ⚪ Minimal · up to cf09c

No actionable issue is established for the spinner documentation update; merge after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the documentation scope and the main change: correcting the hierarchy of the spinner components page.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/ds/DsKit.tsx`:
- Around line 146-151: Update the description layout around DsDescription in
DsKit.tsx to use an explicit spacing utility on the appropriate wrapper when
separation from the title is required; remove reliance on blank JSX lines for
visual spacing. Keep the existing controls gap-1 styling unchanged.

In `@src/routes/ds.spinner.tsx`:
- Around line 19-20: Update the Route.head metadata in src/routes/ds.spinner.tsx
to use the collection title “Spinners” and a description covering both Spinner
and Headbanger, matching the visible page content.
🪄 Autofix

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: fa9ada46-ec72-44d5-8743-5440f52ee449

📥 Commits

Reviewing files that changed from the base of the PR and between e95911b and ec96b3a.

📒 Files selected for processing (2)
  • src/components/ds/DsKit.tsx
  • src/routes/ds.spinner.tsx

Comment thread src/components/ds/DsKit.tsx Outdated
Comment on lines +146 to +151

{description ? (
<DsDescription variant="preview">{description}</DsDescription>
) : null}
</div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use layout utilities for visual spacing.

These added blank lines do not change the rendered layout. JSX source whitespace does not create margin or gap. If the description needs separation from the title, add a spacing class to the wrapper. The controls already have gap-1 at Line 153, so Line 168 is only source formatting.

Also applies to: 168-168

🤖 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 146 - 151, Update the description
layout around DsDescription in DsKit.tsx to use an explicit spacing utility on
the appropriate wrapper when separation from the title is required; remove
reliance on blank JSX lines for visual spacing. Keep the existing controls gap-1
styling unchanged.

Comment thread src/routes/ds.spinner.tsx Outdated
Comment on lines +19 to +20
title="Spinners"
description="A collection of TanStack spinners produced by us for the community."

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Update the page metadata for the new collection.

The visible page now uses title="Spinners" and documents both Spinner and Headbanger, but Route.head still uses the singular title and description. Update the metadata so browser and search metadata describe the full collection.

🤖 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.spinner.tsx` around lines 19 - 20, Update the Route.head
metadata in src/routes/ds.spinner.tsx to use the collection title “Spinners” and
a description covering both Spinner and Headbanger, matching the visible page
content.

@changeset-bot

changeset-bot Bot commented Oct 20, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: cf09cef

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@tannerlinsley

Copy link
Copy Markdown
Member

Thanks Harry, the separate sections and source links make this page easier to use. The contribution is now updated against main, with matching page metadata and the extra introduction removed.

AI review notes:
The final change is limited to the spinner page. DsKit keeps its existing layout, since blank JSX lines do not create visual spacing. The metadata covers both Spinner and Headbanger, and each section retains its sizing, color and reduced-motion guidance.
Rendered an isolated fixture using the actual page body, DsKit, spinner components and site stylesheet. Checked heading hierarchy, source links, both previews and the Spinner code disclosure. This did not exercise the full site shell or production route.
Full pnpm test passed, including type checks, lint, unit, chat and desktop tests. Fresh CI on the updated head is still required before merging.

@tannerlinsley
tannerlinsley merged commit 2ede98d into TanStack:main Oct 20, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants