Skip to content

change(developer): move touch layout editor to typescript - #16520

Open
mcdurdin wants to merge 5 commits into
change/developer/touch-layout-reorganize-filesfrom
change/developer/touch-layout-editor-typescript
Open

change(developer): move touch layout editor to typescript#16520
mcdurdin wants to merge 5 commits into
change/developer/touch-layout-reorganize-filesfrom
change/developer/touch-layout-editor-typescript

Conversation

@mcdurdin

@mcdurdin mcdurdin commented Sep 3, 2026

Copy link
Copy Markdown
Member

A change in threefive commits. It will be easiest to review these changes commit-by-commit and with whitespace changes ignored.

User Testing

TEST_REGRESSION: Open the touch layout editor, press F12 to view Developer Console. Run through all the standard operations of the touch layout editor and verify no errors in Developer Console and that all operations work correctly.

Things to test:

  • edit key properties
  • add/delete keys, rows, layers, platforms, flicks, longpress, multitap etc
  • drag + drop keys to rearrange
  • drag + drop characters from character map

You may wish to compare behaviour against the existing alpha release version.

@github-project-automation github-project-automation Bot moved this to Todo in Keyman Sep 3, 2026
@keymanapp-test-bot keymanapp-test-bot Bot added has-user-test user-test-required User tests have not been completed labels Sep 3, 2026
@keymanapp-test-bot

keymanapp-test-bot Bot commented Sep 3, 2026

Copy link
Copy Markdown

User Test Results

Test specification and instructions

Retesting Template
Test-bot: retest TEST_REGRESSION

Test Artifacts

@github-actions github-actions Bot added developer/ developer/ide/ change Minor change in functionality, but not new labels Sep 3, 2026
@keymanapp-test-bot keymanapp-test-bot Bot added this to the B19S1 milestone Sep 3, 2026
@mcdurdin
mcdurdin marked this pull request as draft September 3, 2026 14:40
@mcdurdin
mcdurdin force-pushed the change/developer/touch-layout-editor-typescript branch 3 times, most recently from b5f6a2b to 321c0fa Compare September 5, 2026 03:55
@mcdurdin
mcdurdin force-pushed the change/developer/touch-layout-editor-typescript branch 2 times, most recently from 5d01473 to 0fb009b Compare September 7, 2026 04:33
@mcdurdin
mcdurdin force-pushed the change/developer/touch-layout-reorganize-files branch from 2d198a3 to 76b36e8 Compare September 7, 2026 04:35
Minimal changeover to esm, use tsc to build. Builds and runs. Installer
will still include source files and the editor should be moved outside
of the xml/ folder later.
Note: doesn't build, just for history
Make minimal changes to Touch Layout Editor source files in order to
build with Typescript with noImplicitAny=false (to be cleaned up later).
Also make source files accessible through the web server for Developer
Console use.
Take the @types/jquery and @types/jqueryui packages out of the
devDependencies and place the index.d.[c]ts files into the ext folder.
This is needed because of fragility elsewhere with implicitly imported
types, which is described for resolution in #16535. Keeping this as a
separate commit as preference would be to have these dependencies
managed through npm long-term.

See-also: #16535
@mcdurdin
mcdurdin force-pushed the change/developer/touch-layout-editor-typescript branch from 0fb009b to d86b2b5 Compare September 7, 2026 04:50
…ayout editor

In order to resolve initialization order (see instigating property
`builder.lastPresentations`), split all constants into a separate object
`builderConstants`, inverting the dependency, which is both cleaner and
the first step of removing the multi-module initialization of a single
`builder` variable.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Unused source file, detected during renames

@mcdurdin
mcdurdin marked this pull request as ready for review September 7, 2026 06:04
@Meng-Heng

Meng-Heng commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Test Specs

  1. Keyman Developer 19.0.281-alpha-test-16520

Test Results

  • TEST_REGRESSION (FAILED):
  1. Open the touch layout editor
  2. Press F12 to view Developer Console
  3. Drag and drop from character map does not work
    image
  4. Remove 'symbol' layer shows errors
    image
  5. After removing the layer but it did not delete, switching the platform and layer had stopped working
    image
  6. Drag and move keys around does not work
    image
    image

@keymanapp-test-bot keymanapp-test-bot Bot added user-test-failed and removed user-test-required User tests have not been completed labels Sep 9, 2026

@jahorton jahorton left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Might want to fix or explicitly defer the TODO entries, though.

KEYMANCORE_PDB=keymancore-2.pdb

function do_build_touch_layout_editor() {
# TODO: this could be a configure step but leaving it here while changes are in flux

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I assume there is a plan to handle these TODOs in a separate PR, given they're still in the incoming changeset?

Comment on lines +79 to +80
# todo: make this an internal dependency
do_build_touch_layout_editor

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TODO:

Comment on lines 210 to +221
begin
doc := ARequestInfo.Document;
if doc.StartsWith('/app/source/toucheditor/lib/') and (Pos('..', doc) = 0) then
if doc.StartsWith('/app/source/toucheditor/src/') and (Pos('..', doc) = 0) then
begin
Delete(doc, 1, Length('/app/source/toucheditor/lib/'));
Delete(doc, 1, Length('/app/source/toucheditor/src/'));
RespondFile(GetLayoutBuilderPath + 'src/' + doc, AContext, ARequestInfo, AResponseInfo);
end
else if doc.StartsWith('/app/source/toucheditor/lib/') and (Pos('..', doc) = 0) then
begin
Delete(doc, 1, Length('/app/source/toucheditor/lib/'));
RespondFile(GetLayoutBuilderPath + 'build/' + doc, AContext, ARequestInfo, AResponseInfo);
end

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems a touch WET, but I do see the distinction between src/ and build/ for the RespondFile statements.

Furthermore, it appears that the lib entries refer to the built versions of the TS files? Basically, anything with a lib path redirects to the TS build output?

The new src entries thus refer to the original, foundational TS source, then?

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

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

3 participants