Fix language selector invalid value #2635
Conversation
🦋 Changeset detectedLatest commit: 210079f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for astro-starlight ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Oh, interesting, I did not checked the deploy preview yet but indeed, this works in dev but not in the production version. I wonder why the difference 🤔 I'll investigate a bit more as it would be nice to avoid JS for this if possible somehow. |
|
I think that the production preview works properly, because it works on my machine :) I'm using Arc v1.29.0 on Windows 11 and here you can see the comparison between current Starlight (left) and preview of this PR (right). Production.works.I.think.mp4PS: I'm sorry that my screen recorder didn't capture the dropdown menu, what are you guys using for recording? |
|
After investigating a bit more and finding so many cases/bug reports regarding how the I'll have to update this PR to use a JS based approach instead when I get the time.
Depending on the machine I use, I'll either use ScreenFlow or the built-in macOS screen recording tool. |
|
Turns out after more investigation that in this specific case, I updated the PR with some code to hook to the I made an E2E test for it while adding the code but I did not submit it:
|
| @@ -46,4 +46,18 @@ function localizedPathname(locale: string | undefined): string { | |||
| } | |||
| } | |||
| customElements.define('starlight-lang-select', StarlightLanguageSelect); | |||
| window.addEventListener('pageshow', (event) => { | |||
There was a problem hiding this comment.
A better event would be resume from the Document API but it's not available in Safari at the moment.
| const markupSelectedIndex = | ||
| select.querySelector<HTMLOptionElement>('option[selected]')?.index; | ||
| if (markupSelectedIndex !== select.selectedIndex) { | ||
| select.selectedIndex = markupSelectedIndex ?? 0; |
There was a problem hiding this comment.
| select.selectedIndex = markupSelectedIndex ?? 0; | |
| select.selectedIndex = markupSelectedIndex ?? select.selectedIndex; |
Please correct me if I'm wrong, but wouldn't it be better to leave the dropdown as it is if markupSelectedIndex is undefined?
Or formulated differently: Why did you fallback to the default locale in this case, HiDeoo?
This is just another option to consider, you are probably right to default to 0...
There was a problem hiding this comment.
If the selected index from the markup is not defined, we're in the case of the default locale with no selected attribute in the markup. For example, if you check the markup on https://starlight.astro.build/, you can see that there is no selected attribute for "English" which makes the first option automatically selected.
Falling back to 0 ensures that the first option is selected in this case even if select.selectedIndex has a different value due to a cache restoration.
Basically, when a cache is restored, the markup is always the source of truth and we never fallback to select.selectedIndex which may not be reliable.
This issue fixes an issue where the language selector could have an invalid value when the user navigates back to a page in a different language.
Repro:
http://localhost:4321/At expected at this point, the page is in English however the language selector is still set to French.
In Chrome & Safari, a refresh of the page would fix the issue. However, in Firefox, a refresh of the page is not enough to fix the issue, you have to manually focus the address bar and press enter again.
This is not something visible with the theme selector as we have some JS code to update the picker when the page is loaded.
Setting the to
autocompleteattribute tooffon theselectelement prvent the browser to automatically select a cached value for this field based on the last user input. Our<Select>component is only used for the theme and theme selector so I applied the change to the component itself.