fix: mobile toc issue with custom banners by trueberryless · Pull Request #3382 · withastro/starlight

trueberryless · 2025-08-15T08:34:19Z

Description

This PR fixes an issue with the Table of Contents being unable to find any heading when the page has a banner with custom styling. This issue is currently tracked in #3047.

The issue is fixed by setting the current heading to the page title, when not found otherwise.

Related stuff and todo

Closes Mobile ToC issue with custom banners #3047
Remove TODOs (therefore the PR is a draft)

changeset-bot · 2025-08-15T08:34:23Z

🦋 Changeset detected

Latest commit: 5c487f5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@astrojs/starlight	Patch

Not sure what this means? Click here to learn what changesets are.

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

netlify · 2025-08-15T08:34:25Z

✅ Deploy Preview for astro-starlight ready!

Name	Link
🔨 Latest commit	`5c487f5`
🔍 Latest deploy log	https://app.netlify.com/projects/astro-starlight/deploys/69726d5d8b0cf6000867cc27
😎 Deploy Preview	https://deploy-preview-3382--astro-starlight.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 100 (no change from production) Accessibility: 100 (no change from production) Best Practices: 100 (no change from production) SEO: 100 (no change from production) PWA: - View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

astrobot-houston · 2025-08-15T08:34:52Z

Lunaria Status Overview

🌕 This pull request will trigger status changes.

Learn more

By default, every PR changing files present in the Lunaria configuration's files property will be considered and trigger status changes accordingly.

You can change this by adding one of the keywords present in the ignoreKeywords property in your Lunaria configuration file in the PR's title (ignoring all files) or by including a tracker directive in the merged commit's description.

Tracked Files

Locale	File	Note
en	getting-started.mdx	Source changed, localizations will be marked as outdated.

Warnings reference

Icon	Description
🔄️	The source for this localization has been updated since the creation of this pull request, make sure all changes in the source have been applied.

delucis

Thanks for having a go at this @trueberryless!

this issue could also be resolved in some other ways of course and I'm not sure if introducing a second IntersectionObserver is a good practice just for this edge case. Feel free to suggest cleaner solutions!

I think you might be right here — we can probably aim to fix the underlying issue if we can.

I tried adding https://github.com/pomber/intersection-observer-debugger to the page which is a handy way to visualize what is happening when working with IntersectionObserver and took a screenshot. This shows the problem quite clearly: the area we track using the rootMargin (highlighted at the top in purple) and the page title (highlighted below in green) don’t intersect:

A Starlight page. An area at the top is highlighted in purple, indicating the area being monitored by the intersection observer. Below is a green heading which is being tested for intersection. They do not intersect.

If we look at a page without the banner, we can see these areas intersect (yellow highlight), triggering the initial ToC state:

Another Starlight page. A similar rectangle at the top is highlighted in purple, but now the page title is highlighted in yellow because it falls within that rectangle.

The correct fix is probably to see if we can make the getRootMargin() method correctly detect an appropriate position by taking the banner into account:

starlight/packages/starlight/components/TableOfContents/starlight-toc.ts

Lines 100 to 110 in 266eed4

    
           private getRootMargin(): `-${number}px 0% ${number}px` { 
        
           	const navBarHeight = document.querySelector('header')?.getBoundingClientRect().height || 0; 
        
           	// `<summary>` only exists in mobile ToC, so will fall back to 0 in large viewport component. 
        
           	const mobileTocHeight = this.querySelector('summary')?.getBoundingClientRect().height || 0; 
        
           	/** Start intersections at nav height + 2rem padding. */ 
        
           	const top = navBarHeight + mobileTocHeight + 32; 
        
           	/** End intersections `53px` later. This is slightly more than the maximum `margin-top` in Markdown content. */ 
        
           	const bottom = top + 53; 
        
           	const height = document.documentElement.clientHeight; 
        
           	return `-${top}px 0% ${bottom - height}px`; 
        
           }

delucis · 2025-09-15T15:45:33Z

Ah, although re-reading your second considered solution is related to that same idea. Hmm. I’ll have to think about that.

trueberryless · 2025-09-16T04:08:24Z

Ah, although re-reading your second considered solution is related to that same idea. Hmm. I’ll have to think about that.

Yeah exactly, if we include the height of the banner in the getRootMargin() calculation, it solves this issue, but introduces a much more annoying issue - when scrolling down the page, headings that do not yet hit the top of the page are already considered "current headings" in the ToC because of the bigger root margin.

As I think that this problem would be much harder to solve with potentially more edge cases, I gave up on the idea quite early, but maybe there is potential 🤔

Also just to mention it if (maybe I forgot to say): It's impossible to change the rootMargin Fter we intitially set it, as mentioned on MDN:

The rootMargin read-only property of the IntersectionObserver interface is a string with syntax similar to that of the CSS margin property.

Notice the read-only.

delucis · 2025-09-18T10:40:31Z

It's impossible to change the rootMargin Fter we intitially set it

Yeah that’s correct, that’s why we destroy and recreate the observer on window resizes currently.

delucis

Thanks again for the PR @trueberryless and for helping us find even more problems 😄

Left a suggestion for the direction to take this.

packages/starlight/components/TableOfContents/starlight-toc.ts

trueberryless · 2026-01-15T17:28:42Z

I will remove the red banner (TODO) in the "Getting started" guide once this PR is approved.
For now, I will move it to "Ready for review" 👍

HiDeoo · 2026-01-15T18:24:27Z

Isn't this related to found being reset every time in the intersection observer callback?

trueberryless · 2026-01-16T06:52:37Z

Isn't this related to found being reset every time in the intersection observer callback?

That was it, thanks for the heads up!

trueberryless · 2026-01-22T12:58:57Z

Now that #3656 is merged, should we also add some test cases to prevent this bug from happening again?

delucis · 2026-01-22T12:59:55Z

Now that #3656 is merged, should we also add some test cases to prevent this bug from happening again?

That would be great! Can you see how to do it or do you need any pointers?

trueberryless · 2026-01-22T13:03:08Z

That would be great! Can you see how to do it or do you need any pointers?

I small pointer in the right direction would actually be very useful to me.

Mainly:

Would you also add them to basics.test.ts?
Would you use your testTOCHighlighting function as well or shall I create a new one to emulate a bigger banner?

delucis · 2026-01-22T13:07:56Z

Would you also add them to basics.test.ts?

I would. But you may want to add a new page to the basics fixture that includes the custom banner.

Would you use your testTOCHighlighting function as well or shall I create a new one to emulate a bigger banner?

I think that function should work, yeah! Just pass it the path to your page with the custom banner.

trueberryless · 2026-01-22T13:17:27Z

For some reason, when I run pnpm test:e2e locally, it get's stuck. So I pushed to see the CI run here

I use Zed on MacOS (still on v18, not v26)... But let's discuss this bug somewhere else 👍

Edit: This is probably because I run Node in a Nix shell where playwright struggles to start either Chrome or Firefox... This should be fixable on my end by modifying some env variables in the Nix shell 🙌

Suggested-by: Chris Swithinbank <swithinbank@gmail.com> Inspired-by: HiDeoo <494699+HiDeoo@users.noreply.github.com>

Co-authored-by: HiDeoo <494699+HiDeoo@users.noreply.github.com>

trueberryless · 2026-01-22T13:35:19Z

Sorry for the force push, but I found some completely unrelated commits in this PR (I don't know how I managed to get them in here), which messed up the labels, so I cleaned up the history a little bit (even if everything get's squashed nonetheless). Now at least the commits look clean here 🥳

delucis

Thanks for the tests @trueberryless! Left a couple of comments/questions.

.changeset/weak-spiders-itch.md

packages/starlight/components/TableOfContents/starlight-toc.ts

Co-authored-by: Chris Swithinbank <357379+delucis@users.noreply.github.com>

delucis

I ended up pushing an entirely different strategy for this fix!

So here’s my thinking: while reviewing the change and thinking about that code repetition, I asked myself why this is an issue to start with? Why do we hit the issue at all. And the answer is “because the banner is too tall, so it pushes the observed elements out of the way”.

Until now we were thinking “OK, we can make a special case, just on page load, where we force the current item to be the page title”.

But in my last round of fixes we already introduced a similar case: we force the page title when walking up the tree and finding the .sl-markdown-content container.

So I thought, “Why does a similar thing not work here?” And long story short, it does! I added the banner (actually any direct child of <main> just in case someone adds e.g. a banner that doesn’t use the sl-banner class name) to the items to observe for intersections and also added them to the page title escape hatch. Seems to work well!

delucis

I believe this is ready, so approving, but will wait for your comment @trueberryless before merging in case you spot something I missed given the code change is quite different now.

trueberryless

Awesome! 🥳

I can't approve my own PR, but I would love to with this clean solution.
Overall everything seems reasonable to me, just left a small question.

trueberryless · 2026-01-22T18:51:32Z

packages/starlight/components/TableOfContents/starlight-toc.ts

-				// Short circuit if we reach the top-level content container.
-				if (el.classList.contains('sl-markdown-content')) {
+				// Short circuit if we reach the top-level content container or one of the other containers in main.
+				if (el.matches('.sl-markdown-content, main > *')) {


I don't fully understand why the addition of main > * is necessary just for the banner? Couldn't we restrict more to avoid unexpected matches or is this intentional?

This is what I was trying to explain in this part of my comment:

any direct child of <main> just in case someone adds e.g. a banner that doesn’t use the sl-banner class name

In other words, we could check for .sl-banner specifically, but we support overriding the banner component and people could use a different component without that class name, OR add some other additional above the page title. And we want to cover both those cases, so that’s why I preferred a more generic selector.

In practice this cannot match most elements in <main> because we exclude most of them in the selector that determines what to watch.

This PR contains the following updates: | Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) | |---|---|---|---| | [@astrojs/starlight](https://starlight.astro.build) ([source](https://github.com/withastro/starlight/tree/HEAD/packages/starlight)) | [`0.37.3` → `0.37.4`](https://renovatebot.com/diffs/npm/@astrojs%2fstarlight/0.37.3/0.37.4) | ![age](https://developer.mend.io/api/mc/badges/age/npm/@astrojs%2fstarlight/0.37.4?slim=true) | ![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@astrojs%2fstarlight/0.37.3/0.37.4?slim=true) | --- ### Release Notes <details> <summary>withastro/starlight (@astrojs/starlight)</summary> ### [`v0.37.4`](https://github.com/withastro/starlight/blob/HEAD/packages/starlight/CHANGELOG.md#0374) [Compare Source](https://github.com/withastro/starlight/compare/@astrojs/starlight@0.37.3...@astrojs/starlight@0.37.4) ##### Patch Changes - [#3534](withastro/starlight#3534) [`703fab0`](withastro/starlight@703fab0) Thanks [@HiDeoo](https://github.com/HiDeoo)! - Fixes support for running builds when `npx` is unavailable. Previously, Starlight would spawn a process to run the Pagefind search indexing binary using `npx`. On platforms where `npx` isn’t available, this could cause issues. Starlight now runs Pagefind using its Node.js API to avoid a separate process. As a side effect, you may notice that logging during builds is now less verbose. - [#3656](withastro/starlight#3656) [`a0e6368`](withastro/starlight@a0e6368) Thanks [@delucis](https://github.com/delucis)! - Fixes several edge cases in highlighting the current page heading in Starlight’s table of contents - [#3663](withastro/starlight#3663) [`00cbf00`](withastro/starlight@00cbf00) Thanks [@lines-of-codes](https://github.com/lines-of-codes)! - Adds Thai language support - [#3658](withastro/starlight#3658) [`ac79329`](withastro/starlight@ac79329) Thanks [@delucis](https://github.com/delucis)! - Avoids adding redundant `aria-current="false"` attributes to sidebar entries - [#3382](withastro/starlight#3382) [`db295c2`](withastro/starlight@db295c2) Thanks [@trueberryless](https://github.com/trueberryless)! - Fixes an issue where the mobile table of contents is unable to find the first heading when a page has a tall banner. </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: Renovate Bot <renovate@zarantonello.dev> Co-committed-by: Renovate Bot <renovate@zarantonello.dev>

* main: [ci] release (withastro#3659) fix: mobile toc issue with custom banners (withastro#3382) chore(deps): update actions/checkout action to v6.0.2 (withastro#3670) Fix table of contents intersection observer (withastro#3656) chore(deps): update peter-evans/create-pull-request action to v8.1.0 (withastro#3669) Pagefind CLI → Pagefind API (withastro#3534) chore(deps): update changesets/action action to v1.6.0 (withastro#3668) i18n(de): Fix orthographic errors in `getting-started.mdx` and `index.mdx` (withastro#3664) i18n(fr): update `resources/plugins.mdx` (withastro#3665) [ci] format i18n(th): Add Thai translations to Starlight UI (withastro#3663) i18n(de): update resources/plugins.mdx (withastro#3662) i18n(ko): update community plugins (withastro#3661)

github-actions bot added 📚 docs Documentation website changes 🌟 core Changes to Starlight’s main package labels Aug 15, 2025

github-actions bot added 🚨 action Changes to GitHub Action workflows 🌟 tailwind Changes to Starlight’s Tailwind package labels Aug 19, 2025

delucis reviewed Sep 15, 2025

View reviewed changes

delucis added this to the v0.38 milestone Dec 2, 2025

delucis mentioned this pull request Jan 15, 2026

Fix table of contents intersection observer #3656

Merged

delucis requested changes Jan 15, 2026

View reviewed changes

packages/starlight/components/TableOfContents/starlight-toc.ts Outdated Show resolved Hide resolved

trueberryless marked this pull request as ready for review January 15, 2026 17:28

This comment was marked as resolved.

Sign in to view

trueberryless requested a review from delucis January 16, 2026 06:53

trueberryless and others added 5 commits January 22, 2026 14:31

fix: mobile toc issue with custom banners

e3d4049

docs: changeset

1e42932

refactor: simplify by just setting current to h1 when nothing intersects

2db3c22

Suggested-by: Chris Swithinbank <swithinbank@gmail.com> Inspired-by: HiDeoo <494699+HiDeoo@users.noreply.github.com>

fix: do not reset found variable

ab8b932

Co-authored-by: HiDeoo <494699+HiDeoo@users.noreply.github.com>

docs: fix typo in changeset

9ac79d0

trueberryless added 2 commits January 22, 2026 14:31

fix: remove todos

3a6b4f4

test: add big banner test case for ToC

24e107f

trueberryless force-pushed the tbl-fix-3047 branch from 35e120b to 24e107f Compare January 22, 2026 13:33

delucis reviewed Jan 22, 2026

View reviewed changes

.changeset/weak-spiders-itch.md Outdated Show resolved Hide resolved

packages/starlight/components/TableOfContents/starlight-toc.ts Outdated Show resolved Hide resolved

packages/starlight/components/TableOfContents/starlight-toc.ts Outdated Show resolved Hide resolved

trueberryless and others added 2 commits January 22, 2026 19:03

fix: suggestions

f53dd66

Co-authored-by: Chris Swithinbank <357379+delucis@users.noreply.github.com>

Change strategy to include banner in watched elements

5c487f5

delucis reviewed Jan 22, 2026

View reviewed changes

delucis added the 🌟 patch Change that triggers a patch release label Jan 22, 2026

delucis removed this from the v0.38 milestone Jan 22, 2026

delucis approved these changes Jan 22, 2026

View reviewed changes

delucis removed 🚨 action Changes to GitHub Action workflows 🌟 tailwind Changes to Starlight’s Tailwind package 📚 docs Documentation website changes labels Jan 22, 2026

trueberryless commented Jan 22, 2026

View reviewed changes

delucis merged commit db295c2 into withastro:main Jan 23, 2026
17 checks passed

astrobot-houston mentioned this pull request Jan 23, 2026

[ci] release #3659

Merged

trueberryless deleted the tbl-fix-3047 branch January 23, 2026 16:53

	private getRootMargin(): `-${number}px 0% ${number}px` {
	const navBarHeight = document.querySelector('header')?.getBoundingClientRect().height \|\| 0;
	// `<summary>` only exists in mobile ToC, so will fall back to 0 in large viewport component.
	const mobileTocHeight = this.querySelector('summary')?.getBoundingClientRect().height \|\| 0;
	/** Start intersections at nav height + 2rem padding. */
	const top = navBarHeight + mobileTocHeight + 32;
	/** End intersections `53px` later. This is slightly more than the maximum `margin-top` in Markdown content. */
	const bottom = top + 53;
	const height = document.documentElement.clientHeight;
	return `-${top}px 0% ${bottom - height}px`;
	}

Uh oh!

Conversation

trueberryless commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related stuff and todo

Uh oh!

changeset-bot bot commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

netlify bot commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for astro-starlight ready!

Uh oh!

astrobot-houston commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Lunaria Status Overview

Tracked Files

Uh oh!

delucis left a comment

Choose a reason for hiding this comment

Uh oh!

delucis commented Sep 15, 2025

Uh oh!

trueberryless commented Sep 16, 2025

Uh oh!

delucis commented Sep 18, 2025

Uh oh!

delucis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

trueberryless commented Jan 15, 2026

Uh oh!

This comment was marked as resolved.

HiDeoo commented Jan 15, 2026

Uh oh!

trueberryless commented Jan 16, 2026

Uh oh!

trueberryless commented Jan 22, 2026

Uh oh!

delucis commented Jan 22, 2026

Uh oh!

trueberryless commented Jan 22, 2026

Uh oh!

delucis commented Jan 22, 2026

Uh oh!

trueberryless commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

trueberryless commented Jan 22, 2026

Uh oh!

delucis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

delucis left a comment

Choose a reason for hiding this comment

Uh oh!

delucis left a comment

Choose a reason for hiding this comment

Uh oh!

trueberryless left a comment

Choose a reason for hiding this comment

Uh oh!

trueberryless Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

delucis Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

trueberryless commented Aug 15, 2025 •

edited

Loading

changeset-bot bot commented Aug 15, 2025 •

edited

Loading

netlify bot commented Aug 15, 2025 •

edited

Loading

astrobot-houston commented Aug 15, 2025 •

edited

Loading

trueberryless commented Jan 22, 2026 •

edited

Loading

delucis Jan 23, 2026 •

edited

Loading