Sidebar doesn't hide on navigation on non-English languages on mobile #2020

New Issue

WofWca · 2023-03-08T12:11:43Z

WofWca commented

2023-03-08 12:11:43 +00:00

Description

How to reproduce:

Go to /user/settings/general
Set language to non-English
Reload the page.
Make the window narrow
Open the sidebar and navigate somewhere

The sidebar didn't close, as it would on English.

That is, this line doesn't work when the language is not DEFAULT_LANGUAGE:

1b06112db4/src/composables/useMenuActive.ts (L38)

It's a race condition. For non-English languages setLanguage doesn't return an immediately resolved promise because it has to import the lang:

373b04bd58/src/i18n/index.ts (L47-L49)

setLanguage is called here:

4be53b098c/src/main.ts (L110-L112)

Which results in useMenuActive getting executed before the app has been mounted, resulting in useRoute returning undefined here:

1b06112db4/src/composables/useMenuActive.ts (L19)

This also completely breaks the app in dev mode (this is how I discovered it, in fact).

Vikunja Frontend Version

0eb78e32

Vikunja API Version

v0.20.1+150-4f62e978ef

Browser and version

No response

Can you reproduce the bug on the Vikunja demo site?

Yes

Screenshots

No response

### Description How to reproduce: 1. Go to `/user/settings/general` 2. Set language to non-English 3. Reload the page. 4. Make the window narrow 5. Open the sidebar and navigate somewhere The sidebar didn't close, as it would on English. That is, this line doesn't work when the language is not `DEFAULT_LANGUAGE`: https://kolaente.dev/vikunja/frontend/src/commit/1b06112db4ba5ad4144b5868dd04e954be1d77f7/src/composables/useMenuActive.ts#L38 It's a race condition. For non-English languages `setLanguage` doesn't return an immediately resolved promise because it has to import the lang: https://kolaente.dev/vikunja/frontend/src/commit/373b04bd58a16e4415a1be4a2c53267d44662d11/src/i18n/index.ts#L47-L49 `setLanguage` is called here: https://kolaente.dev/vikunja/frontend/src/commit/4be53b098ca909194aefb464a93b6dae99f4b9ab/src/main.ts#L110-L112 Which results in `useMenuActive` getting executed before the app has been mounted, resulting in `useRoute` returning `undefined` here: https://kolaente.dev/vikunja/frontend/src/commit/1b06112db4ba5ad4144b5868dd04e954be1d77f7/src/composables/useMenuActive.ts#L19 This also completely breaks the app in dev mode (this is how I discovered it, in fact). ### Vikunja Frontend Version 0eb78e32 ### Vikunja API Version v0.20.1+150-4f62e978ef ### Browser and version _No response_ ### Can you reproduce the bug on the Vikunja demo site? Yes ### Screenshots _No response_

👍 1

WofWca added the

kind/bug

label 2023-03-08 12:11:43 +00:00

konrad commented

2023-03-08 13:39:32 +00:00

Huh. Very interesting find. I was able to reproduce this but had to reload the window, it seems to work fine immediately after changing the language. Thanks for all the debug info!

Basically, once we fix the promise return here to make sure it always only returns when the language is fully loaded it should work?

Huh. Very interesting find. I was able to reproduce this but had to reload the window, it seems to work fine immediately after changing the language. Thanks for all the debug info! Basically, once we fix the promise return [here](https://kolaente.dev/vikunja/frontend/src/commit/373b04bd58a16e4415a1be4a2c53267d44662d11/src/i18n/index.ts#L47-L49) to make sure it always only returns when the language is fully loaded it should work?

konrad commented

2023-03-08 13:40:18 +00:00

Or maybe we can fix the route watcher? Why does that actually return undefined when the language is not loaded? Aren't these two completely separate parts?

WofWca commented

2023-03-08 14:18:04 +00:00

had to reload the window

Right, forgot to mention that.

Basically, once we fix the promise return here to make sure it always only returns when the language is fully loaded it should work?

Not sure what you mean. Isn't it the case now?

Why does that actually return undefined

I think it's about the app not being mounted. useRoute warns (in dev build)

inject() can only be used inside setup() or functional components.

> had to reload the window Right, forgot to mention that. > Basically, once we fix the promise return here to make sure it always only returns when the language is fully loaded it should work? Not sure what you mean. Isn't it the case now? > Why does that actually return undefined I think it's about the app not being mounted. `useRoute` warns (in dev build) > inject() can only be used inside setup() or functional components.

konrad commented

2023-03-08 14:57:32 +00:00