Skip to content

fix: mobile toc issue with custom banners #3382

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

fix: mobile toc issue with custom banners #3382

wants to merge 5 commits into from
+39 −0

Conversation

trueberryless
Copy link
Contributor

@trueberryless trueberryless commented Aug 15, 2025 •
edited
Loading

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 introducing a temporary IntersectionObserver without any rootMargin (so that it definitely gets an intersection target). This new code only runs when there is no other intersecting target found by the existing IntersectionObserver.

Visit https://deploy-preview-3382--astro-starlight.netlify.app/getting-started/ and make the screen a smaller to see the mobile ToC, then reload and see that a current heading is set.

Please note that 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!

Another considered solution

Another solution that came to my mind, but where I figured that it has too many side effects (and therefore I didn't commit to it): By dynamically incorporating the actual height of the banner in the calculation of bottom it would be possible to always guarantee that a heading can be found (which would mean that we don't need a second observer). However, the problem with that approach would be that the observer would then permanently be too big and headings would "too early" be considered current headings. For example: If the banner is really big, headings that scroll into the middle of the screen could potentially be set as the "current" headings in the ToC, which would be misleading.

To demonstrate what I mean with this paragraph in code:

-const bottom = top + 53;
+const bottom = top + 53 + bannerHeight; // assuming we calculate the actual banner height before

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

Line 107 in 2768932

const bottom = top + 53;

Related stuff and todo

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Aug 15, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: fe3b4cb

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 Netlify
Copy link

netlify bot commented Aug 15, 2025 •
edited
Loading

✅ Deploy Preview for astro-starlight ready!

Name Link
🔨 Latest commit fe3b4cb
🔍 Latest deploy log https://app.netlify.com/projects/astro-starlight/deploys/68a45a4d1720ef00085c0e3f
😎 Deploy Preview https://deploy-preview-3382--astro-starlight.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
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.

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

astrobot-houston commented Aug 15, 2025 •
edited
Loading

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.

trueberryless and others added 2 commits August 15, 2025 10:34
@trueberryless
Merge branch 'main' into tbl-fix-3047
e455d7a
@hippotastic @delucis
@HiDeoo @trueberryless
Add basic ESLint config, lint script and lint CI job (withastro#1640)
28a7923 
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
Co-authored-by: HiDeoo <494699+HiDeoo@users.noreply.github.com>
Co-authored-by: delucis <357379+delucis@users.noreply.github.com>
@github-actions github-actions bot added 🚨 action Changes to GitHub Action workflows 🌟 tailwind Changes to Starlight’s Tailwind package labels Aug 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
🚨 action Changes to GitHub Action workflows 🌟 core Changes to Starlight’s main package 📚 docs Documentation website changes 🌟 tailwind Changes to Starlight’s Tailwind package
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Mobile ToC issue with custom banners
3 participants
@trueberryless @astrobot-houston @hippotastic