agents icon indicating copy to clipboard operation
agents copied to clipboard

docs: do not hide 'on this page' text in table of contents

Open dimaMachina opened this issue 1 month ago • 3 comments

I don't think we should hide this toc header

image

dimaMachina avatar Nov 25 '25 10:11 dimaMachina

⚠️ No Changeset found

Latest commit: 81800533a163944d8cab920fe8d1fa1c300d8b0a

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.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

changeset-bot[bot] avatar Nov 25 '25 10:11 changeset-bot[bot]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
agents-docs Ready Ready Preview Comment Nov 25, 2025 10:47am
agents-manage-api Error Error Nov 25, 2025 10:47am
agents-manage-ui Ready Ready Preview Comment Nov 25, 2025 10:47am
agents-run-api Ready Ready Preview Comment Nov 25, 2025 10:47am
agents-test Ready Ready Preview Comment Nov 25, 2025 10:47am

vercel[bot] avatar Nov 25 '25 10:11 vercel[bot]

Claude finished @dimaMachina's task —— View job


PR Review: Remove "On this page" text hiding in table of contents

Todo List

  • [x] Read the changed CSS file to understand what was removed
  • [x] View the provided screenshot to understand visual impact
  • [x] Analyze the change from UX and accessibility perspectives
  • [x] Review system design and consistency considerations
  • [x] Provide comprehensive feedback and recommendations

Summary

This PR removes a CSS rule that was hiding the "On this page" heading in the table of contents (#nd-toc h3 { display: none; }). The change is straightforward but raises several important considerations.

Technical Analysis

What was changed:

  • Removed CSS selector #nd-toc h3 with display: none from agents-docs/src/app/global.css:125-127
  • This targets the heading element within Fumadocs' table of contents component

Impact:

  • The "On this page" text will now be visible above the table of contents items
  • No breaking changes to functionality - purely visual enhancement

Review Feedback

Positive Aspects

  1. Accessibility Improvement: Making the TOC heading visible improves semantic structure and screen reader navigation
  2. Clean Change: Simple, focused change that addresses a specific UI issue
  3. User Experience: The screenshot shows this creates better visual hierarchy and context for users

🔍 Areas for Consideration

1. Design System Consistency (Medium Priority)

The Fumadocs framework likely includes this heading for a reason. Consider:

  • Is this hiding intentional across other Fumadocs implementations?
  • Should this be addressed at the design system level rather than overriding?
  • Check if other documentation sites using similar patterns show or hide this heading

2. Changeset Requirements (High Priority)

The changeset-bot flagged that this change needs a changeset:

  • This is a user-facing change that affects the visual appearance
  • Should include a changeset for @inkeep/agents-docs package
  • Recommended changeset type: patch (minor visual improvement)

3. Responsive Design Verification (Medium Priority)

The screenshot appears to be from a desktop view:

  • Verify how this looks on mobile devices
  • Ensure the heading doesn't cause layout issues in narrow viewports
  • The mobile TOC styling at #nd-tocnav (line 121-124) may need consideration

4. Documentation Context (Low Priority)

Consider documenting why this override was removed:

  • Was the original hiding intentional for a specific design reason?
  • Future maintainers should understand the decision rationale

🚀 Recommendations

Immediate Actions Required:

  1. Add a changeset - This is blocking the PR and required for proper versioning
  2. Test mobile responsiveness - Verify the change works well on smaller screens

Suggested Improvements:

  1. Consider adding a comment explaining why the default Fumadocs behavior is preferred:

    /* mobile toc */
    #nd-tocnav {
      /* ... existing styles ... */
    }
    /* Note: Fumadocs default "On this page" heading is shown for better UX */
    
  2. Alternative approach: If there are specific design concerns about the heading, consider styling it instead of hiding it:

    #nd-toc h3 {
      font-size: 0.875rem;
      color: var(--muted-foreground);
      margin-bottom: 0.5rem;
    }
    

📋 Changeset Suggestion

---
"@inkeep/agents-docs": patch
---

Show "On this page" heading in table of contents for improved navigation clarity

Overall Assessment

This is a sound improvement that enhances user experience and accessibility. The change aligns with standard documentation patterns and the reasoning is solid. The main blocker is the missing changeset, which should be added before merging.

Recommendation: Approve after adding changeset

claude[bot] avatar Nov 25 '25 10:11 claude[bot]

done in https://github.com/inkeep/agents/pull/1131

dimaMachina avatar Dec 08 '25 17:12 dimaMachina