Added menus to access Keyboard Shortcuts Modal #6833
Conversation
akshayasalvi
changed the title
|
@roryabraham @parasharrajat While all the changes are done, I am stuck at one issue. For the system menu -> View Keyboard Shortcuts
Tried with |
|
@akshayasalvi there is another issue to change the keyboard shortcut to open the modal to |
|
@roryabraham PR updated |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to production by @sketchydroide in version: 1.1.22-2 🚀
|
|
🚀 Deployed to staging by @roryabraham in version: 1.1.22-2 🚀
|
| // Defines the system-level menu item for manually triggering an update after | ||
| const updateAppMenuItem = new MenuItem({ | ||
| label: 'Update New Expensify', | ||
| enabled: false, | ||
| click: quitAndInstallWithUpdate, | ||
| }); | ||
|
|
||
| // Defines the system-level menu item for opening keyboard shortcuts modal | ||
| const keyboardShortcutsMenu = new MenuItem({ | ||
| label: 'View Keyboard Shortcuts', |
There was a problem hiding this comment.
Is this a visible label? If so, this is not correct (and really the same can be said about the one right above this one).
All texts need to be localized to the current user's language.
There was a problem hiding this comment.
it would be a bit of a challenge to add the localization to the Main process (native layer desktop) as Onyx data is stored on the renderer process UI layer.
We can communicate between them.
Crazy thought! should we parse the user locale from the OS timezone settings for the desktop app too?
There was a problem hiding this comment.
We can communicate between them.
Let's do that then? Is it super hard or what?
Crazy thought! should we parse the user locale from the OS timezone settings for the desktop app too?
I thought we already did, like where does JS's Intl get the data from? I assume that in the end, they come from the OS, no? Especially for the desktop app?
There was a problem hiding this comment.
Let's do that then? Is it super hard or what?
Nope, But it seems reversed, For example, first the main app will be initialized then the UI layer so options will render in English and then switch from English to Selected language. But this time would be very short so it seems fine.
I thought we already did, like where does JS's Intl get the data from? I assume that in the end, they come from the OS, no? Especially for the desktop app?
As far I can tell we don't parse that. we depend on the timezone from the user's profile which I think defaults to geolocation.
There was a problem hiding this comment.
I checked the other menu items like "Update New Expensify", "History", etc. they weren't localized.
We can communicate between them.
Can give it a try, will have to check that when the User changes the language preference, the system menu should update?
There was a problem hiding this comment.
As far I can tell we don't parse that. we depend on the timezone from the user's profile which I think defaults to geolocation.
Oh right. I was confused with something different. Right now we don't detect language at all. Maybe in the future when we focus on actually releasing a localized app, we can do it, for now it is not worth it.
Can give it a try, will have to check that when the User changes the language preference, the system menu should update?
Yes, it should, but the more I look at this the more I think we should do that in a separate issue.
|
🚀 Deployed to production by @yuwenmemon in version: 1.1.23-1 🚀
|
|
🚀 Deployed to production by @yuwenmemon in version: 1.1.23-1 🚀
|
Details
Changes for keyboard shortcuts
Fixed Issues
$ #6660
Tests
QA Steps
Steps for About Menu
Steps for Desktop Menu
Tested On
Screenshots
Web
Mobile Web
NA
Desktop
iOS
NA
Android
NA