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

Language Select incorrectly appends locale when switching index pages #1423

Closed
1 task
kevinzunigacuellar opened this issue Jan 28, 2024 · 5 comments · Fixed by #1442
Closed
1 task

Language Select incorrectly appends locale when switching index pages #1423

kevinzunigacuellar opened this issue Jan 28, 2024 · 5 comments · Fixed by #1442
Labels
🐛 bug Something isn't working good first issue Good for newcomers

Comments

@kevinzunigacuellar
Copy link
Sponsor Member

kevinzunigacuellar commented Jan 28, 2024

What version of starlight are you using?

0.17

What version of astro are you using?

4.2.6

What package manager are you using?

pnpm

What operating system are you using?

Mac

What browser are you using?

Chrome

Describe the Bug

Using different locales (en,es) and setting my build to format file. My index page is located in en.html and es.html respectively.

When switching from one language to the other using the select element, it incorrectly adds a locale in front of my route instead of replacing the route.

Current behavior ❌

When going from es to en:

es.html -> en/es.html

When going from en to es:

en.html -> es/en.html

Expected behavior ✅

en.html -> es.html and viceversa

Link to Minimal Reproducible Example

https://github.com/kevinzunigacuellar/bug-language-pick

Participation

  • I am willing to submit a pull request for this issue.
@delucis
Copy link
Member

delucis commented Jan 29, 2024

Thanks for finding this @kevinzunigacuellar! I can confirm I see this in the build output of your reproduction. The language picker produces the following HTML:

<select value="/en/en.html">
	<option value="/en/en.html" selected="true">English</option>
	<option value="/es/en.html">Spanish</option>
</select>

value is set here:

value={localizedPathname(Astro.props.locale)}

So this would be a bug in the localizedPathname() function.

Are you up for Even More Fun With URLs™️?

@delucis delucis added 🐛 bug Something isn't working good first issue Good for newcomers labels Jan 29, 2024
@kevinzunigacuellar
Copy link
Sponsor Member Author

Happy to tackle this if we have no other takers. I will leave it open if anyone is interested.

@delucis
Copy link
Member

delucis commented Jan 29, 2024

Thanks @kevinzunigacuellar! I tried writing a few tests on the train earlier and the one thing I realised I'm not sure about with sites using build.output: 'file' is what they do for the homepage, like is it example.com/index.html? Or would they expect to special case that as example.com/? I'm honestly not sure what expectations are.

@kevinzunigacuellar
Copy link
Sponsor Member Author

kevinzunigacuellar commented Jan 29, 2024

I think they would expect example.com/index.html. example.com/ would probably redirect to example.com/index.html

@delucis
Copy link
Member

delucis commented Jan 29, 2024

I think we can definitely take that as our working hypothesis.

I'm still feeling like our use of build.format as also determining link format may not be what everyone wants, but I think it's ok for us to get this consistent at this point and think about that later when we hear from folks what they need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants