Skip to content

Conversation

@jahorton
Copy link
Contributor

@jahorton jahorton commented Oct 2, 2025

After changes to be added in #14880, one of our unit tests will fail without this change. After lots of investigation, it turns out that the isSubstitutionAlignable function does not catch certain cases that determineContextSlideTransform does. The latter does need some improvement to work as a fix, but a small change lets it pick up the slack and replace the former.

As it's also used during context-state transition, re-using the same method in both places will give us far more confidence that we won't have an error the second time around.

Build-bot: skip build:web
Test-bot: skip

After changes added in #14880, one of our unit tests failed.  After lots of investigation, it turns out that the `isSubstitutionAlignable` does not catch certain cases that `determineContextSlideTransform` does.  The latter does need some improvement to work as a fix, but a small change lets it pick up the slack and replace the former.

As it's also used during context-state transition, re-using the same method in both places gives us far more confidence that we won't have an error the second time around.

Build-bot: skip build:web
Test-bot: skip
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Oct 2, 2025

User Test Results

Test specification and instructions

User tests are not required

@keymanapp-test-bot keymanapp-test-bot bot changed the title fix(web): fix base context-state validation fix(web): fix base context-state validation 🚂 Oct 2, 2025
@keymanapp-test-bot keymanapp-test-bot bot added this to the A19S13 milestone Oct 2, 2025
jahorton added a commit that referenced this pull request Oct 3, 2025
WIth the changes in #14883, we are now safe to remove the fallback check that was the one place still utilizing ContextTracker.analyzeState.  As a result... time for more dead code removal.

Build-bot: skip build:web
Test-bot: skip
jahorton added a commit that referenced this pull request Oct 7, 2025
WIth the changes in #14883, we are now safe to remove the fallback check that was the one place still utilizing ContextTracker.analyzeState.  As a result... time for more dead code removal.

Build-bot: skip build:web
Test-bot: skip
@jahorton jahorton marked this pull request as ready for review October 8, 2025 16:51
@darcywong00 darcywong00 modified the milestones: A19S13, A19S14 Oct 11, 2025
Base automatically changed from feat/web/interim-legacy-keyer to epic/autocorrect October 16, 2025 14:49
@jahorton jahorton merged commit 15e0096 into epic/autocorrect Oct 16, 2025
7 of 8 checks passed
@jahorton jahorton deleted the fix/web/base-context-state-validation branch October 16, 2025 15:36
@github-project-automation github-project-automation bot moved this from Todo to Done in Keyman Oct 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants