-
-
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): Add more non-printing characters #9846
Conversation
User Test ResultsTest specification and instructions
Test Artifacts
|
@@ -199,6 +201,22 @@ function TransformSpecialKeys14(FDebug: boolean, sLayoutFile: string): string { | |||
return sLayoutFile; | |||
} | |||
|
|||
function TransformSpecialKeys17(FDebug: boolean, sLayoutFile: string): string { |
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.
A bit WET, based on TransformSpecialKeys14 (ll 188-202)
Hm, from @bharanidharanj 's screenshot, the new characters show tofu. I thought Developer would be using the same keymanweb-osk.ttf font. |
…hore/developer/non-printing-chars
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.
Changes look good, except we may still have the old font?
Co-authored-by: Marc Durdin <[email protected]>
I copied keymanweb-osk.ttf to developer. (didn't realize it's a separate copy) @keymanapp-test-bot retest |
developer/src/test/auto/kmcomp/test_touchlayout_14_1.keyman-touch-layout
Outdated
Show resolved
Hide resolved
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.
Need to revert the three test_touchlayout_14 files and then I think it should be okay with the proposed changes
common/web/types/src/kmx/kmx.ts
Outdated
public static readonly VERSION_110 = 0x00000B00; | ||
public static readonly VERSION_120 = 0x00000C00; | ||
public static readonly VERSION_130 = 0x00000D00; |
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.
We shouldn't add these because we don't have files with these version numbers
for(let i = 0; i < CSpecialText17Map.length; i++) { | ||
// Assumes the JSON output format will not change | ||
if(FDebug) { | ||
sLayoutFile = sLayoutFile.replaceAll('"text": "'+CSpecialText17Map[i][0]+'"', '"text": this._v>16 ? "'+CSpecialText17Map[i][0]+'" : "'+CSpecialText17Map[i][1]+'"'); | ||
} else { | ||
sLayoutFile = sLayoutFile.replaceAll('"text":"'+CSpecialText17Map[i][0]+'"', '"text":this._v>16?"'+CSpecialText17Map[i][0]+'":"'+CSpecialText17Map[i][1]+'"'); | ||
} | ||
} |
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.
We are using a multidimensional array here instead of a map, due to the conversion from Delphi. One day should clean this up
Co-authored-by: Marc Durdin <[email protected]>
Co-authored-by: Marc Durdin <[email protected]>
@mcdurdin - I think all the test files got reverted. Is this ready to merge now? |
LGTM |
Changes in this pull request will be available for download in Keyman version 17.0.207-alpha |
Continues #9547
This adds special characters to Developer that were added in the new KeymanWeb font.
User Testing
Setup - Install the PR build of Keyman Developer
(will be in this list)
keyman/web/src/engine/osk/src/specialCharacters.ts
Lines 46 to 67 in a4e081a