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

Add more OS keyboard layouts #1165

Merged
merged 21 commits into from
Sep 19, 2022
Merged

Conversation

precondition
Copy link
Contributor

@precondition precondition commented Aug 19, 2022

Description

Generated keycode LUTs for conversion of the configurator front-end from one OS keyboard layout to another with the help of the convert_keymap_extras_header.js script added in #1161.

@precondition precondition marked this pull request as ready for review September 4, 2022 10:41
This is done so that the Shift+GESC and GUI+GESC keysyms/keycode names
remain on the same line because the `breakLines` functions of
PrintKey.vue and TesterKey.vue replace spaces ' ' with newlines '\n'
@@ -79,10 +79,67 @@ const state = {
{ value: 'zh-CN', label: '简体中文' }
],
osKeyboardLayouts: [
'keymap_belgian',
Copy link
Collaborator

Choose a reason for hiding this comment

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

ok this is long enough now, please extract it to it's own JS file and import it. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in commit ffcfc24. Although I'm not sure what's the best directory to put in or what case mode to use for the filename (exclusion_list.js, longFormKeycodes.js, and potato-facts.js all use a different case mode so I don't know what's the desired style for filenames)

Copy link
Contributor Author

@precondition precondition Sep 4, 2022

Choose a reason for hiding this comment

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

Ideally, we wouldn't even need to maintain a list of OS keyboard layout identifiers and just tell the configurator to list all src/i18n/keymap_extras/keymap_*.js files (and strip the path stem and .js extension) but I don't know what would be an idiomatic way to achieve that.

Copy link
Collaborator

@yanfali yanfali Sep 4, 2022

Choose a reason for hiding this comment

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

Ideally, we wouldn't even need to maintain a list of OS keyboard layout identifiers and just tell the configurator to list all src/i18n/keymap_extras/keymap_*.js files (and strip the path stem and .js extension) but I don't know what would be an idiomatic way to achieve that.

We would have to do it at build time and write some node js that ran as part of the build and generated the file. There's some precedence for this, i18n and version number of build. This app is serverless, so it doesn't have any file system as such and you can't really glob a file system when you are over a http request. So better to do it at build time.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Done in commit ffcfc24. Although I'm not sure what's the best directory to put in or what case mode to use for the filename (exclusion_list.js, longFormKeycodes.js, and potato-facts.js all use a different case mode so I don't know what's the desired style for filenames)

if you want to rename all the files to one of the styles for consistency I'd be open to that. I think I'd prefer the snake casing if we were going to pick a style.

@precondition
Copy link
Contributor Author

Bump

@yanfali
Copy link
Collaborator

yanfali commented Sep 12, 2022

This is kind of big. I know it's a lot of configuration. One concern is that it's loading all this stuff into memory. I wonder if we can lazy load this as it's not needed by 90% of users. I'll look at it over the next few days.

@yanfali
Copy link
Collaborator

yanfali commented Sep 17, 2022

Intending to review this Sunday my time. Busy week.

Copy link
Collaborator

@yanfali yanfali left a comment

Choose a reason for hiding this comment

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

What a passion project. Very impressive. As far as I can tell it doesn't seem to impact loading times too much as it's mostly text which should compress well.

I happened to find a feature you might like in the vite docs:

https://vitejs.dev/guide/features.html#glob-import

I think we talked about this once a while back.

@yanfali
Copy link
Collaborator

yanfali commented Sep 19, 2022

@noroadsleft give this a once over please. My only usability peccadillo is that the drop down is now huge. But other than that it's a very cool feature.

@noroadsleft
Copy link
Member

My only usability peccadillo is that the drop down is now huge.

It's not only huge, it's also wide – nearly wider than the settings panel itself, at least for me:

image

Copy link
Member

@noroadsleft noroadsleft left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@noroadsleft noroadsleft merged commit 22bc5a7 into qmk:master Sep 19, 2022
@noroadsleft
Copy link
Member

Thanks!

@precondition
Copy link
Contributor Author

I can look into replacing the drop down with a scrollable dialog box with spoilers for each language.

@precondition precondition deleted the more_os_kb_layouts branch October 5, 2022 07:59
@yanfali
Copy link
Collaborator

yanfali commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants