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

docs: refactor social links configuration reference #869

Merged
merged 7 commits into from
Oct 15, 2023

Conversation

HiDeoo
Copy link
Member

@HiDeoo HiDeoo commented Oct 11, 2023

What kind of changes does this PR include?

  • Changes or translations of Starlight docs site content

Description

As discussed on Discord, this pull request refactors the social links section of the Customizing Starlight guide by removing the complete list of social links and replacing it with a link to the configuration reference where the type is now dynamically generated.

This means that PRs adding new social links will no longer require follow-up i18n PRs.

Notes:

* main:
  Add missing changeset from withastro#852
  i18n(zh-CN): fix another link to astro main doc (withastro#867)
  i18n(zh-CN): fix link to astro main doc (withastro#866)
  [i18nIgnore] i18n(zh-cn): Fix Chinese subtag (withastro#855)
@changeset-bot
Copy link

changeset-bot bot commented Oct 11, 2023

⚠️ No Changeset found

Latest commit: 0201469

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

@netlify
Copy link

netlify bot commented Oct 11, 2023

👷 Deploy Preview for astro-starlight processing.

Name Link
🔨 Latest commit 0201469
🔍 Latest deploy log https://app.netlify.com/sites/astro-starlight/deploys/652bc07cc6e2560008a37508

@github-actions github-actions bot added 📚 docs Documentation website changes 🌟 core Changes to Starlight’s main package labels Oct 11, 2023
@@ -208,7 +208,7 @@ interface BadgeConfig {

### `locales`

**type:** <code>{ \[dir: string\]: [LocaleConfig](#localeconfig) }</code>
**type:** <code>\{ \[dir: string\]: [LocaleConfig](#localeconfig) \}</code>
Copy link
Member Author

Choose a reason for hiding this comment

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

Change required due to the MDX conversion to avoid interpreting this as an expression.

---
import { socialLinks } from '../../../packages/starlight/schemas/social';

const socials = [...socialLinks].sort((a, b) => a.localeCompare(b, 'en'));
Copy link
Member Author

Choose a reason for hiding this comment

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

Relying on localeCompare() in the compare function instead of a basic sort() to ensure a case-insensitive sort so that codeberg appears before codePen in the list for example.

Copy link
Member

Choose a reason for hiding this comment

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

Looks good. Initially, I wondered if we should sort with the current page’s locale instead of always en, but on second thoughts, given this list’s contents, this makes sense 👍

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.

Looks good @HiDeoo! I’ll also try to make sure we merge any other pending translations of the two impacted pages ahead of this.

docs/src/content/docs/guides/customization.mdx Outdated Show resolved Hide resolved
---
import { socialLinks } from '../../../packages/starlight/schemas/social';

const socials = [...socialLinks].sort((a, b) => a.localeCompare(b, 'en'));
Copy link
Member

Choose a reason for hiding this comment

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

Looks good. Initially, I wondered if we should sort with the current page’s locale instead of always en, but on second thoughts, given this list’s contents, this makes sense 👍

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
const socials = [...socialLinks].sort((a, b) => a.localeCompare(b, 'en'));
---

<code>{`Partial<Record<${socials.map((social) => `'${social}'`).join(' | ')}, string>>`}</code>
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

This component is very creative!

@delucis delucis mentioned this pull request Oct 13, 2023
* main: (42 commits)
  docs: refactor sidebar label translations examples to use tags with regional subtags (withastro#881)
  Add Nostalgist.js docs to showcase (withastro#900)
  i18n(id): Fix environmental-impact.md translation (withastro#899)
  i18n(es): Update `customization.mdx`, `i18n.mdx` & `configuration.md` (withastro#897)
  i18n(ko): update `configuration.md` (withastro#896)
  i18n(ko): update `i18n.mdx` (withastro#895)
  i18n(ko): update `customization.mdx` (withastro#894)
  [ci] format
  i18n(id): translate guides/components.mdx and guides/project-structure.mdx (withastro#898)
  [ci] format
  [ci] release (withastro#865)
  Add Patreon social link (withastro#892)
  [ci] format
  Add Ukrainian language support (withastro#891)
  i18n(id): Translate more pages and improve existing translations (withastro#859)
  [ci] format
  feat: add Reddit social icon (withastro#854)
  [ci] format
  i18n(ru): added translations for components, css-and-tailwind, environmental-impact, project-structure pages (withastro#888)
  i18n(id): translate `guides/authoring-content.md` (withastro#863)
  ...
@HiDeoo
Copy link
Member Author

HiDeoo commented Oct 15, 2023

Updated the PR with the changes of #892 & #854.

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.

LGTM! Going to merge this while the coast is clear and we don't have too many other translation PRs just yet. Thanks again @HiDeoo for the great idea.

@delucis delucis merged commit ce9a739 into withastro:main Oct 15, 2023
9 checks passed
vasfvitor added a commit to vasfvitor/astro-starlight that referenced this pull request Oct 21, 2023
improve wording
acima -> em cima
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.

3 participants