Explore how to support internationalization of the keymap #1166
Conversation
When I originally wrote this, await was still pretty new and how to use it wasn't as clear to me as now. Flatten out the changeKeyboard function so it takes advantage of await and remove the callbacks. This should make the code a little bit more deterministic at the cost of using await which uses more resources.
- take advantage of await to flatten callbacks
- move the resolve to after the keymap is done computing
Vue2 specific projects need specific setup for Volar otherwise visual studio displays the wrong linting errors.
In the previous implementation the BaseKey rendering code was doing a dynamic lookup to render the keycode for each key, so that we would correctly render ISO keycodes. This was fine until someone wanted to switch out the keycodes for locale specific keycodes and then it required changes in many places in order for the rendering to be correct for the keycodes. Instead of doing it per key rendering, just sweep all the codes once when we switch locale ISO/ANSI on master and update all the names to the newly corrected names. This will enable preconditions PR which allow support for all the locales that QMK firmware supports in a more scaleable manner that doesn't touch nearly as many places in the UI code.
Hook into toggleIso function so the keyboard tester switches the legends on the tester keyboard dynamically. Just call tester/init to reinit the keyboard layouts.
- remove the toggle for ANSI and ISO and let the global ISO state toggle the UI layout for the tester
- remove node-sass - upgrade sass
- use the settings panel to toggle layout change
yanfali
force-pushed
the
chore-simplify-callbacks
branch
from
0769179 to
2abe297
Compare
| @@ -202,6 +208,11 @@ export default { | |||
| } | |||
| } | |||
| }, | |||
| async mounted() { | |||
| await this.initializeKeyboards(); | |||
There was a problem hiding this comment.
just make all these tasks synchronous, it doesn't make sense to not wait for all of these things to complete
| </button> | ||
| </slot> | ||
| </div> | ||
| <div class="layout-selector-radios"></div> |
There was a problem hiding this comment.
remove buttons but keep layout the same.
| @@ -56,47 +56,43 @@ const actions = { | |||
| * @param {string} keyboard new keyboard we are switching to | |||
| * @return {object} promise that will be fulfilled once action is complete | |||
| */ | |||
| changeKeyboard({ state, commit, dispatch }, keyboard) { | |||
| async changeKeyboard({ state, commit, dispatch }, keyboard) { | |||
There was a problem hiding this comment.
just make this a "synchronous" function
There was a problem hiding this comment.
I'm confused, why does the code say "async" but your comment says the opposite?
There was a problem hiding this comment.
It's the irony of async. It makes async things appear synchronous to the developer. Hence air quotes.
| @@ -22,7 +22,8 @@ describe('Tester feature', function () { | |||
| }); | |||
| it('Should change layout', function () { | |||
| // ISO has 105 keys | |||
| cy.get('.layout-selector-radios > :nth-child(2)').click(); | |||
| cy.get('.bes-controls').click(); | |||
There was a problem hiding this comment.
fix tests for new UI behavior
| * correct ISO support. | ||
| * @param {} state | ||
| */ | ||
| updateKeycodeNames(state) { |
There was a problem hiding this comment.
So this is the core of the new behavior. When requested, force the keymap to update all the keycode names from the latest lookup keycode table. This is what the UI uses to determine what to display. The keycodes don't change between locales. Only the name/label/displayname which is all derived for name. The only special handling here is for container keys, where it's the contained key that needs an update, not the container.
| updateKeycodeNames(state) { | ||
| let store = this; | ||
| // assumes the keycode store has changed due to layout update | ||
| state.keymap = state.keymap.reduce((layers, layer) => { |
There was a problem hiding this comment.
to ensure reactivity works correctly, just change the reference to a new keymap with all the old values copied into it. This will force the UI to re-render.
This is a bit of a mish mash. Working on issues for preconditions new PR for international support I found many smaller issues with the way I wrote this code, before the arrival of await support. Things were very racey before because it was a pain to write this code using callbacks.
Flattens out some fo the callbacks so they are easier to comprehend.
Also cleans up some issues with visual code so that the warnings are now better and removes a really annoying volar issue with vue2/vue3 complaining about something which isn't valid.
Also fixes the way keymap rendering works, which was the original intent of this work.
So in the original ISO/ANSI support contributed, thanks @itsjxck the choice to re-lookup every keycode in the BaseKey was actually just fine because at the time we only supported 2 layouts, ANSI and ISO. @precondition opened a PR which will enable every layout and this exposed a bug in the original implementation which we did not notice.
Basically many rendering decisions are made based on the value of each keys meta data. Specifically meta.name.
when we switch between different locales, this usually wasn't a problem as the name's didn't change much in length. But, in some locales we go away from multi character rendering towards single character rendering. Because some of the CSS applied is using the length of the name to make class decisions, which would result in some keys with single character names getting rendered as if they were multi-character.
Example is KC_SCLN, which in german is
ö, and in ANSI is rendered as:\n;.So this patch changes the logic, so that instead of doing the look up dynamically as we're rendering, when we switch locale just sweep the entire keymap and update all the names with the current name for the current locale.
Why would we do it this way? Well it turns out we also use meta.name is any place we render keys. Like BaseKey, PrintKey and TesterKey.
TODO: fix the keyboard tester so this logic also works.Fixed keyboard tester so it reacts to the ISO toggle.Description