refactor: Remove Sidebar wrapper from TableOfContents component

[ainsleyclark]

Summary

Removed the <Sidebar> wrapper component from TableOfContents, allowing consumers to control the layout wrapper themselves.

Changes

• Removed the Sidebar import from TableOfContents.svelte
• Unwrapped the table of contents markup by removing the <Sidebar> component wrapper
• The core table of contents functionality (heading, menu items, and styling) remains unchanged
• Added changeset documenting this as a patch-level change

Details

This change gives consumers more flexibility in how they structure their layouts. Previously, the TableOfContents component automatically wrapped its content in a Sidebar component. Now, consumers are responsible for providing their own wrapper if needed, allowing for greater composition and layout control.

https://claude.ai/code/session_015fonomvgKkgUUybC6fbrcA

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[claude]

Review summary

• Overall score: 5/10
• Critical issues: 1
• Warnings: 2
• Suggestions: 1
• Recommendation: ❌ Request changes

The change itself is clean and well-scoped, but the semver classification is wrong and migration guidance is absent — both of which will cause pain for downstream consumers.

---

Critical issues 🔴

Changeset version is patch but this is a breaking change

The Sidebar component provides mobile slide-in drawer behaviour, sticky desktop positioning, focus management, keyboard (Escape) dismiss, and an overlay. Removing it from TableOfContents silently breaks all of these for existing consumers on upgrade. By semver this warrants a major bump, not patch.

# .changeset/remove-toc-sidebar-wrapper.md
- "@ainsleydev/sveltekit-helper": patch   ← should be major
+ "@ainsleydev/sveltekit-helper": major

---

Warnings 🟡

No migration guidance

The component docs and changeset give no indication of how to replicate the previous behaviour. A consumer who upgrades will lose the mobile sidebar silently. At a minimum the changeset body and @component JSDoc should show the new expected usage:

<Sidebar>
  <TableOfContents heading="On this page" />
</Sidebar>

contentSelector default still references sidebar semantics

The default contentSelector = '[data-sidebar-content="true"]' and the headingSelector fallback data-sidebar-selector are fine to keep, but they now carry stale naming that implies a sidebar wrapper is always present. Worth noting in docs that these attribute names are independent of the Sidebar component.

---

Suggestions 🟢

Update the @example blocks to reflect the new composition pattern

The existing examples show <TableOfContents /> in isolation, which was fine when Sidebar was implicit. Now that consumers own the wrapper, at least one example should demonstrate wrapping with <Sidebar> to achieve the prior behaviour — otherwise the component appears to have lost responsive layout support entirely.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.26%. Comparing base (7f6b060) to head (41d956e).
⚠️ Report is 520 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #435      +/-   ##
==========================================
+ Coverage   64.59%   70.26%   +5.67%     
==========================================
  Files         154      187      +33     
  Lines        6064     7439    +1375     
==========================================
+ Hits         3917     5227    +1310     
+ Misses       2064     2012      -52     
- Partials       83      200     +117     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.