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

Add more EC text markers and frames to Starlight docs #1102

Merged
merged 7 commits into from
Nov 23, 2023

Conversation

HiDeoo
Copy link
Member

@HiDeoo HiDeoo commented Nov 20, 2023

What kind of changes does this PR include?

  • Changes or translations of Starlight docs site content

Description

This PR is a follow-up to this comment I made in the EC PR and a review I did a while ago regarding all the code blocks in the Starlight documentation to add missing EC text markers and frame informations in a more consistent way.

I edited everything I could find but I think some may be up to discussions, like for example having src/content/docs/example.md as frame title for all examples in the frontmatter reference may not be the best, but as I don't personally know the guidelines for this I think this PR could be used as a base to discuss this even if we end up not merging it or only partially (we could also decide to add this kind of changes only when we next edit the relevant portions of the docs).

Copy link

vercel bot commented Nov 20, 2023

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

Name Status Preview Updated (UTC)
starlight ✅ Ready (Inspect) Visit Preview Nov 22, 2023 5:05pm

Copy link

changeset-bot bot commented Nov 20, 2023

⚠️ No Changeset found

Latest commit: f07fea6

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

@github-actions github-actions bot added the 📚 docs Documentation website changes label Nov 20, 2023
Copy link
Member

@kevinzunigacuellar kevinzunigacuellar left a comment

Choose a reason for hiding this comment

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

Love the Expressive codification of Starlight 💜

docs/src/content/docs/guides/i18n.mdx Outdated Show resolved Hide resolved
Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Awesome work @HiDeoo! Left a few comments but most of this is a really nice improvement — cool to see so quickly after we released support for this.

docs/src/content/docs/manual-setup.mdx Outdated Show resolved Hide resolved
docs/src/content/docs/guides/components.mdx Outdated Show resolved Hide resolved
docs/src/content/docs/guides/components.mdx Show resolved Hide resolved
docs/src/content/docs/guides/customization.mdx Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Curious what other people think here. A lot of these code snippets are fragments like

starlight({
  customCss: ['./src/custom-styles.css', '@fontsource/roboto'],
})

In these cases, I wonder if it’s misleading to add the astro.config.mjs file name because it makes it looks like this is the full file rather than just a part of it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this makes sense and we could definitely remove the frame titles (except the first one).

Altho, if we go with this approach, we have the same pattern in the following files that may need fixing too:

  • docs/src/content/docs/guides/customization.mdx
  • docs/src/content/docs/guides/sidebar.mdx

The question would be if we either remove the frame titles for those too or we add the imports? Or maybe this is only relevant in this specific file?

Copy link
Member

Choose a reason for hiding this comment

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

Scrolling through customization.mdx on the current production version I think that actually looks good yeah — file names are used for full examples, and not for shorter fragments (usually accompanying a longer one like in the “Add your logo” section).

Copy link
Member

Choose a reason for hiding this comment

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

I personally like the startlight() snippets because they are easier to copy and paste. Sometimes I would have a very convoluted astro.config and just having the startlight snippet to copy and paste is very nice.

HiDeoo and others added 2 commits November 22, 2023 11:25
Co-authored-by: Chris Swithinbank <[email protected]>
Co-authored-by: Kevin Zuniga Cuellar <[email protected]>
@HiDeoo
Copy link
Member Author

HiDeoo commented Nov 22, 2023

Updated the PR with the suggested changes and left a comment regarding the configuration.mdx file.

@HiDeoo
Copy link
Member Author

HiDeoo commented Nov 22, 2023

Updated the PR with the following changes:

  • Remove previously introduced frame titles in guides/customization.mdx
  • Remove all frame titles in reference/configuration.mdx except the first code block
  • Remove astro.config.mjs frame titles from guides/sidebar.mdx as it's explicitely mentioned in the text before any code block (I left the src/content/docs/example.md one)

I think this should cover all the feedback we got so far. Let me know if I missed anything or if you have any other suggestions.

Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Thank you @HiDeoo! 🚀

@delucis delucis changed the title Add more EC text markers and frame informations to Starlight docs Add more EC text markers and frames to Starlight docs Nov 23, 2023
@delucis delucis merged commit 4b3143b into withastro:main Nov 23, 2023
7 checks passed
vasfvitor added a commit to vasfvitor/astro-starlight that referenced this pull request Dec 25, 2023
vasfvitor added a commit to vasfvitor/astro-starlight that referenced this pull request Dec 25, 2023
vasfvitor added a commit to vasfvitor/astro-starlight that referenced this pull request Dec 25, 2023
vasfvitor added a commit to vasfvitor/astro-starlight that referenced this pull request Dec 25, 2023
delucis added a commit that referenced this pull request Jan 3, 2024
* `overriding-components.md` #1168

* `getting-started.mdx` (#1241)

* `components.mdx` (#1102)

* `css-and-tailwind.mdx` (#1102)

* `customization.mdx` (#1102)

* `manual-setup.mdx` (#1102)

* translate filename in `components.mdx`

* fix: indentation

Co-authored-by: Chris Swithinbank <[email protected]>

---------

Co-authored-by: Chris Swithinbank <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 docs Documentation website changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants