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

[i18nIgnore] Variant of sidebar PR with buttons #2168

Merged
merged 70 commits into from
May 20, 2024

Conversation

simonhyll
Copy link
Sponsor Contributor

@simonhyll simonhyll commented May 19, 2024

Mainly want the preview to be built to get input from others easily on whether this is a good idea :)

TODO:

  • 🚫 Fix blog posts, which should show and sorting: we'll do this later imo, it's good enough as it is, there's an upstream issue regarding fixing this, if that doesn't arrive in time we have the ability to make a custom workaround but I'd rather wait for the upstream solution
  • ✅ Rearrange files/folders so all the content shows in the sidebar under one of the tabs
  • ✅ Look into hamburger on LP
  • ✅ Search button to the left

@simonhyll simonhyll self-assigned this May 19, 2024
Copy link

socket-security bot commented May 19, 2024

Copy link

netlify bot commented May 19, 2024

Deploy Preview for tauri-v2 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit f13572f
🔍 Latest deploy log https://app.netlify.com/sites/tauri-v2/deploys/664b58ac8ba964000993cc71
😎 Deploy Preview https://deploy-preview-2168--tauri-v2.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: 80 (🟢 up 50 from production)
View the detailed breakdown and full score reports

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

@simonhyll simonhyll marked this pull request as ready for review May 19, 2024 07:06
@simonhyll simonhyll requested a review from a team as a code owner May 19, 2024 07:06
@lorenzolewis
Copy link
Member

There were some CSS fixed I found in mobile Safari you may want to apply: lorenzolewis/starlight-multi-sidebar@5cd2bcf

@vasfvitor
Copy link
Contributor

I will take a look at it, just to have more eyes at it

Copy link
Contributor

@vasfvitor vasfvitor left a comment

Choose a reason for hiding this comment

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

any special reason you changed from
/releases to /release,
/references to /reference,
/concepts to /concept?

Besides those, there's a few other that I think it's important to add to netlify _redirects:
/frontend-configuration                /frontend
/test                                              /develop/tests
/guides/features                            /guides/feature
/features                                        /plugin
/start/upgrade--migrate              /start/migrate

Copy link
Member

@dreyfus92 dreyfus92 left a comment

Choose a reason for hiding this comment

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

Hey @simonhyll, amazing work! I’m also curious about something @vasfvitor mentioned: what was the reasoning behind switching from plural to singular in some routes (e.g., references to reference, concepts to concept, and so on)?

@dreyfus92 dreyfus92 changed the title Variant of sidebar PR with buttons [i18nIgnore] Variant of sidebar PR with buttons May 20, 2024
@simonhyll
Copy link
Sponsor Contributor Author

simonhyll commented May 20, 2024

@vasfvitor @dreyfus92 I changed the folder name from plural to singular because it makes more sense to me in singular when you look at the URL, and because it makes sense to keep all folders in the same format, like, it's called "develop" not "develops", but then it's "guides" not "guide". One can argue it's a folder that contains guides, so it's "guides", but what you're reading is just one "guide", so looking at the URL it'd be "tauri.app/start/migrate/from-tauri-v1" for example instead of "tauri.app/starts/migrations/from-tauri-1", because it'd be a folder of different starting points, and we have multiple forms of migrations/upgrades, but that looks really weird and it's longer, makes more sense to think of it in the URL as "/start" ok, I'm reading a guide about getting started, "/migrate" currently I'm migrating something, and "/from-tauri-v1" ok I'm getting started migrating from tauri v1, as opposed to "i'm in the starting points folder, reading a migration, about tauri v1". Basically using the structure to describe what you're reading as opposed to where you're reading it.

That's at least the sort of logic I used for the change. Keep it uniform, and if we're keeping it uniform I prefer to think of it as descriptions of the page you're reading/doing, not as a folder structure of files. Like making a CLI, you don't have tauri builds because you're using a group of commands related to building, you have tauri build because that's what you're doing right now, you don't tauri builds androids dev because there's multiple things you can do with an android, you have tauri build android dev because you are "building for android in dev mode". Same with a URL, it's "I start a migration from tauri v1", not "I'm reading one of the starts where one of the migrations is about going from v1 to v2"

@simonhyll
Copy link
Sponsor Contributor Author

And sure @vasfvitor I can update the redirects (even though I'd love to not do any of that at all since the site isn't stable yet and people shouldn't be bookmarking stuff before it's released 🙃 but I accept that it's a community favour with little added effort so I'll do it)

@simonhyll
Copy link
Sponsor Contributor Author

simonhyll commented May 20, 2024

I'm not opposed to going back to plural format btw, the most important thing for me is just that the folder names are using a uniform naming conventions, so if we'd change all of them to then it'd be things like "starts", "developers", "migrations"; tests" etc, which to me gets weird for some words, like "develop", are we talking about "developers" because it's for developers, or is it a verb, but if it's a verb how do we do the naming for a verb, is it "develop", "develops", "developing"

Basically, naming is weird and I think thinking of it as descriptions of what you wanna do right now and the contents of the page makes it easier to keep the URL's short and sweet while maintaining logic in the folder structure.

@simonhyll
Copy link
Sponsor Contributor Author

simonhyll commented May 20, 2024

Oh and btw the only reason some folders go against the naming convention of singular, so "Tests" instead of "test", is because Starlight autogenerate doesn't have a feature for setting the name, so the folder is forced to be called what the name should be, which is extremely annoying but something we'd need an upstream fix for.

@simonhyll
Copy link
Sponsor Contributor Author

I added the redirects now and the sidebar fixes from @lorenzolewis, so unless we wanna discuss the folder naming further I think we're good to merge?

@vasfvitor
Copy link
Contributor

thanks, as you said, not much effort to avoid the annoying experience of finding a broken links in search engine results or break the always open tab someone may have.

@simonhyll simonhyll requested a review from dreyfus92 May 20, 2024 14:49
Copy link
Member

@dreyfus92 dreyfus92 left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for the clarification, @simonhyll! Let's go ahead and ship this. We can always iterate and make changes based on future feedback 😁

@simonhyll simonhyll merged commit 7661418 into tauri-apps:v2 May 20, 2024
7 checks passed
@simonhyll simonhyll deleted the feat/no-splash-with-buttons branch May 20, 2024 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants