refactor(web): refactor and harmonize constants 🏗️ #12072
Conversation
User Test ResultsTest specification and instructions User tests are not required Test Artifacts
|
ermshiperete
force-pushed
the
change/web/8146_moveCodes2
branch
from
acb65c1 to
007ada3
Compare
web/src/app/browser/build.sh
Outdated
| @@ -13,6 +13,7 @@ SUBPROJECT_NAME=app/browser | |||
| # ################################ Main script ################################ | |||
|
|
|||
| builder_describe "Builds the Keyman Engine for Web's website-integrating version for use in non-puppeted browsers." \ | |||
| "@/common/web/types build" \ | |||
There was a problem hiding this comment.
If this has already been added by @/common/web/keyboard-processor, including it here is redundant. That's why there's no @/common/web/keyboard-processor listed in the original version - that, in turn, is auto-included via input-processor, then engine/main - the last of which is listed.
Granted, the fact that we are directly importing from it in a few files may be worth keeping it listed, I guess.
| "@/common/web/types build" \ |
There was a problem hiding this comment.
For comparison, you didn't add a listing for web/src/engine/main/build.sh, despite it also not listing @/common/web/keyboard-processor as a dependency.
|
|
||
| const Codes = { | ||
| // Define Keyman Developer modifier bit-flags (exposed for use by other modules) | ||
| // Compare against /common/include/kmx_file.h. CTRL+F "#define LCTRLFLAG" to find the secton. | ||
| modifierCodes: { |
There was a problem hiding this comment.
| modifierCodes: { | |
| // Debug-mode keyboards compiled before Keyman 18.0 referenced the `ModifierKeyConstants` | |
| // constants via the names established below. We must continue to support them, as they're | |
| // essentially part of the keyboard API now. | |
| modifierCodes: { |
Optionally, we could also emit the same values under their new name:
| modifierCodes: { | |
| // Debug-mode keyboards compiled before Keyman 18.0 referenced the `ModifierKeyConstants` | |
| // constants via the names established below. We must continue to support them, as they're | |
| // essentially part of the keyboard API now. | |
| modifierCodes: { | |
| ...ModifierKeyConstants, |
so that at some point in the distant future, we could drop the old names entirely... maybe.
ermshiperete
force-pushed
the
change/web/8146_moveCodes2
branch
2 times, most recently
from
c54de67 to
3ce5f14
Compare
Note that this adds the keyCodes > 50000 to `USVirtualKeyCodes`. Fixes: #8146
ermshiperete
force-pushed
the
change/web/8146_moveCodes2
branch
from
3ce5f14 to
2c2acbe
Compare
| "SHIFT":0x0010, // K_SHIFTFLAG | ||
| "CTRL":0x0020, // K_CTRLFLAG | ||
| "ALT":0x0040, // K_ALTFLAG | ||
| ...ModifierKeyConstants, |
There was a problem hiding this comment.
I don't think we should include this here -- we should maintain this as a legacy interface only. Mixing the names in a published interface doesn't buy us anything -- we have to maintain the old names anyway.
…odes` Keep `modifierCodes` as legacy API. This addresses a code review comment (#12072 (comment)).
|
Changes in this pull request will be available for download in Keyman version 18.0.83-alpha |
Fixes: #8146
@keymanapp-test-bot skip