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

feat: Create Directory.astro component #1717

Merged
merged 46 commits into from
Jan 2, 2024

Conversation

vasfvitor
Copy link
Contributor

What kind of changes does this PR include?

  • Minor content fixes (broken links, typos, etc.)
  • Changes to the docs site code

Description

This is updated so It can be both be used in the current situation and in the migration list or elsewhere.
For reference: #1670 (comment)

Usage:

<Features path="features" locale="en" />

I chose for now to make locale mandatory so it's clear that it is needed on translated pages. With a bit of more work it could be removed by matching path to locales.json but first I want to put this out here to gather feedback on which direction should this go.

Currently, on i18n pages, first is rendered translated cards and then fallback pages, excluding translated. Another possibility is to leave them mixed, in which Notifications card would be down below.

"(en)" is for clarity, but I do think something to indicate it's fallback is nice to have.

Preview:

<Features path="fr/guides/develop/" locale="fr" />

Another use case:

<Features path="/guides/develop/" locale="en" />

I think before merging, it should be decided this will be the same component in the aforementioned issue comment or change it's name.

Thank you.

Copy link

netlify bot commented Nov 29, 2023

Deploy Preview for tauri-docs-starlight ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 91b923b
🔍 Latest deploy log https://app.netlify.com/sites/tauri-docs-starlight/deploys/658c45069c10440008375ad6
😎 Deploy Preview https://deploy-preview-1717--tauri-docs-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: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions github-actions bot added the i18n Is this related to translations? label Nov 29, 2023
@lorenzolewis
Copy link
Member

Have a few thoughts on this but going to put them in Discord for us to chat through when you're online: https://discord.com/channels/616186924390023171/662624589037436928/1179737597925392434

@vasfvitor vasfvitor changed the title feat: Update Features.astro to handle i18n and receive props feat: Create Directory.astro component Nov 30, 2023
@lorenzolewis
Copy link
Member

I wonder if this should also respect order in https://starlight.astro.build/reference/frontmatter/#sidebarconfig?

@vasfvitor
Copy link
Contributor Author

I wonder if this should also respect order in https://starlight.astro.build/reference/frontmatter/#sidebarconfig?

good point, yes

@github-actions github-actions bot removed the i18n Is this related to translations? label Dec 13, 2023
@vasfvitor vasfvitor marked this pull request as draft December 13, 2023 04:48
@vasfvitor
Copy link
Contributor Author

DO NOT MERGE

Updates:

  • Removed locales props, whether path it's localized or not, the component forces current page locale.
  • Sort following sidebar order, with option to override it

Anything else?

Copy link
Member

@lorenzolewis lorenzolewis left a comment

Choose a reason for hiding this comment

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

Just put a few general feedback comments. Overall pretty happy with this. There's a few moving parts upstream with the Badge in a LinkCard, but happy to merge this when you're ready and just refactor when those land.

src/components/list/Directory.astro Outdated Show resolved Hide resolved
src/components/list/Directory.astro Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the i18n Is this related to translations? label Dec 19, 2023
@lorenzolewis
Copy link
Member

Happy with this overall. I think the final piece is putting it in place on the respective pages in this PR so we can double-check that we're happy with the results.

@vasfvitor
Copy link
Contributor Author

vasfvitor commented Dec 20, 2023

had to add a filter by filename, how do you feel of having both options(filterOutByFileName and filterOutByTitle)? Was thinking of making a more complex filter but there's no need yet, I'd say.

@lorenzolewis
Copy link
Member

I would do slug to roughly match what Content Collections can do: https://docs.astro.build/en/guides/content-collections/#filtering-collection-queries

@vasfvitor
Copy link
Contributor Author

done, anything else?

@lorenzolewis
Copy link
Member

Looks good. I think the references usages need to be updated with the latest in next as those URLs were changed with #1762

@lorenzolewis
Copy link
Member

Awesome! Let's get this merged in 💪

@lorenzolewis lorenzolewis merged commit a35b24e into tauri-apps:next Jan 2, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants