-
-
Notifications
You must be signed in to change notification settings - Fork 111
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
feat(developer): cleanup touch layout files on load and save 🙊 #6917
feat(developer): cleanup touch layout files on load and save 🙊 #6917
Conversation
User Test ResultsTest specification and instructions User tests are not required Test Artifacts
|
8182dad
to
1cb0444
Compare
Improves the .keyman-touch-layout clean schema and adds additional cleanup during load and save in the Touch Layout Editor. The compiler already does transforms for the relevant fields to the formats that KeymanWeb is expecting (in particular, width, pad to string). This is preparation for adding hint data to the format as well.
5bf1235
to
60dabb5
Compare
1cb0444
to
51a94a1
Compare
"anyOf": [ | ||
{"required": ["id"]}, | ||
{"required": ["sp"]}, | ||
{"required": ["sk"]}, | ||
{"required": ["flick"]}, | ||
{"required": ["multitap"]} | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there cases where we may wish to support an id-less key that has subkeys, flicks, or multitaps?
Granted, I'm not super-great with schemas, but my impression of this part of the spec appears to permit such a case.
I know we permit id-less keys with sp
properties - such as for a 'blank' space for row padding, so no complaints on that one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your reading is correct.
I guess it seems unlikely but not impossible? A 'special characters' key which has no default behaviour but does have flicks or longpress -- where the keyboard author doesn't want it to be triggered by accident? Like I say, unlikely but probably no really good reason to restrict the schema.
return true; | ||
}; | ||
|
||
let cleanupNumericField = function(obj, prop) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a case where this can return false
- only true
. Granted, it is part of a boolean-logic condition chain in a different function, but it still strikes me a bit odd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given none of the cleanup*
functions are returning false
I could make them conceptually void
functions. BUt not sure if it makes any difference? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still feels a bit dirty to me as it is (see previous comments), but I suppose the complaints are a bit nitpicky. I can go with "good enough."
ca47fce
into
feat/developer/touch-layout-compiler-flick-and-multitap
Improves the .keyman-touch-layout clean schema and adds additional cleanup during load and save in the Touch Layout Editor.
The compiler already does transforms for the relevant fields to the formats that KeymanWeb is expecting (in particular, width, pad to string).
This is preparation for adding hint data to the format as well.
@keymanapp-test-bot skip (automated tests will cover this with compiler regression testing)