Skip to content
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.

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

css: move overflow wrap rule to markdown.css #1885

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

julien-deramond
Copy link
Contributor

⚠️ This PR is a draft


Description

This PR is a follow-up of what's been discussed in #1736 (review)

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.

Basically, we move the p ,h1, h2, h3, h4, h5, h6, code { overflow-wrap: anywhere; } rule to the markdown.css.

Temporarily, the red code shows the elements impacted by the previous rule, and the green one shows the elements impacted by the new rule.

Copy link

changeset-bot bot commented May 17, 2024

⚠️ No Changeset found

Latest commit: 243d8b8

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

Copy link

vercel bot commented May 17, 2024

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

Name Status Preview Updated (UTC)
starlight ✅ Ready (Inspect) Visit Preview May 17, 2024 7:24am

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

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.

@@ -40,7 +40,7 @@ h4,
h5,
h6,
code {
overflow-wrap: anywhere;
background-color: rgba(255, 0, 0, 0.2);
Copy link
Contributor Author

@julien-deramond julien-deramond May 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been through the entire doc, the only elements that are displayed in red (old rule) are the page main titles, and the "On this page" area (see https://starlight-git-fork-julien-deramond-main-jd-dc421d-astrodotbuild.vercel.app/getting-started/).

Screenshot 2024-05-17 at 09 26 05

The search message error is probably not important, and the main title as well:

Screenshot 2024-05-17 at 09 30 28

Screenshot 2024-05-17 at 09 31 16

The rest in green works well.

One solution would probably be to keep that in markdown.css and add specific rules for this main title and "On this page" area. What do you think @delucis? Want me to try this approach in this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌟 core Changes to Starlight’s main package 📚 docs Documentation website changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants