fix: prevent list items from overflowing Markdown content

[julien-deramond]

---

⚠️ If this is not going to right direction, feel free to close the PR directly. If this is the right solution, please don't forget to remove:

• the reddish background
• the additional fake content

---

Description

Context

This PR is an attempt to fix an issue reported in this Discord > starlight > Text overflow issue thread.

On iPhone XR, OP could see on a 414px width screen the following (at https://0d6fe6ee.chart-docs.pages.dev/charts/dependency/kube-state-metrics/) which results in a horizontal scrollbar:

It can also be observed, at least on macOS with Safari and Chrome.

Changes

As you can see in the deployed documentation, I've tested some long links in the regular content, but also in ordered and unordered lists. Regular content is fine, while the long links in the lists are overflowing.

The idea here is to rely on the same fixes that happened in https://github.com/withastro/starlight/pull/756 and https://github.com/withastro/starlight/pull/814 based on the use of overflow-wrap: anywhere.

Not knowing in detail the codebase of Starlight, I suppose that reducing the scope to .sl-markdown-content li is acceptable (I've added a reddish background for visual manual regression testing). Indeed, adding this rule directly in the reset.css might have some unexpected repercussions.

Here's the rendering before the fix.

Here's the rendering after the fix (directly accessible at https://starlight-git-fork-julien-deramond-main-jd-bab67d-astrodotbuild.vercel.app/guides/authoring-content/):

Non-regression testing

I'm sorry but I haven't tested yet some impacts that would maybe need a stricter CSS rule, and I'll be off for 1 week tomorrow.

Some use cases that could be impacted:

• https://starlight-git-fork-julien-deramond-main-jd-bab67d-astrodotbuild.vercel.app/getting-started/: tabs if there are too much of them
• https://starlight-git-fork-julien-deramond-main-jd-bab67d-astrodotbuild.vercel.app/guides/project-structure/#example-project-contents: directories list with some long names or too many sub-directories
• https://starlight-git-fork-julien-deramond-main-jd-bab67d-astrodotbuild.vercel.app/guides/css-and-tailwind/#custom-css-styles: complex lists with graphical elements in them
• https://starlight-git-fork-julien-deramond-main-jd-bab67d-astrodotbuild.vercel.app/guides/customization/#add-your-logo: combo of previous mentioned use cases
• https://starlight-git-fork-julien-deramond-main-jd-bab67d-astrodotbuild.vercel.app/resources/community-content/#astro-videos: list of cards

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: b82f34de2c3717f272282ba0e1a368979c56b5be

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

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
starlight	✅ Ready (Inspect)	Visit Preview	May 16, 2024 8:48pm

[julien-deramond]

I guess we’ll need to override the overflow-wrap: anywhere for that component. It also gives a small indication that this could have an impact on user components in theory.

Yes, if it is difficult to measure the impact on Starlight side, we can be sure that it will have an impact on users' components.

It makes me wonder if our global rule setting this should be scoped to the Markdown content and use :not(:where(.not-content *)) like the other rules to make it easier for users to escape that styling if needed. This rule here:

https://github.com/withastro/starlight/blob/main/packages/starlight/style/reset.css#L35-L44

My first version was adding li to this rule, but it was a little bit "scary" not to be able to measure the impact and the potential regressions for the users, not knowing all the details of the codebase.

I'd be in favor to scope it to the Markdown content. It would seem to be the most maintainable approach for the Starlight core team, and maybe also less opinionated and/or impactful for the users and their components, yeah.

[HiDeoo]

Thanks for the great contribution. We reviewed this PR with a lot of eyes/devices/browsers today in Astro Talking & Doc'ing session.

To do so, we had everyone browse random pages on various Starlight websites (Starlight Docs, Astro Docs, SST Ion docs and Knip Docs) where I implemented ahead of time the suggested fix from this PR.

With exactly the suggested changes from this PR, no extra issue except the one spotted by Chris regarding the <Tabs> component was found.

We tried a slight variation of the selector (.sl-markdown-content li:not(:where(.not-content *))) which prevents this issue as the tabs list wrapper uses the not-content class. Altho, as spotted during the session, this would not work in the case of the <Tabs> component nested in the <Steps> component for example.

Did not play more than that yet with the selector, but I guess it will need a bit more tweaking to handle those cases.

[astrobot-houston]

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.
en	guides/authoring-content.md	Source changed, localizations will be marked as outdated.
en	guides/customization.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.

[julien-deramond]

Sorry for the delay, I was off and also overwhelmed by other OSS repos.

Based on https://github.com/withastro/starlight/pull/1736#issuecomment-2064296251 and https://github.com/withastro/starlight/pull/1736#pullrequestreview-1992414929, I've reset the overflow-wrap value for tab items via https://github.com/withastro/starlight/pull/1736/commits/d5fae7d2f18bbac4b5e58b8f3a0ebb9a480423c1#diff-07167ef1e6f2c6cadc53adf10a29f1e6057bcfad18c10b956fbfa6626bf43f15.

This can be tested out here:

• Start Here > Getting Started with the long "Yarnnnnn[...]" string in a basic <TabItem>
• Guides > Customizing Starlight > Set up fonts > Set up a Fontsource font with the long "Yarnnnnn[...]" string in a <Steps> + <TabItem>

With these modifications, the PR should cover all use cases.

[julien-deramond]

I do wonder if our generic set-up for this in reset.css ought really also to be moved to markdown.css: In a quick check, I think the only element this currently applies to which is outside of the Markdown content is the page title. But that can happen as follow-up work to keep this PR focused on the specific fix.

Yes, I can take a look in a follow-up PR to ease the review and evaluate the impacts.

Whenever this PR is accepted, feel free to ping me so that I can remove the extra testing data before merging :)

[julien-deramond]

Tried to add one thanks to your doc via https://github.com/withastro/starlight/pull/1736/commits/b82f34de2c3717f272282ba0e1a368979c56b5be. Chose "patch" bump. Feel free to modify it so that it's consistent with the other changesets :) I'm not that aware of your architecture and release management.