Skip to content

refactor: extract ShiftHandler from GeneralKeyboardIME (Part 13) - #426 - #688

Open
prince-0408 wants to merge 1 commit into
scribe-org:mainfrom
prince-0408:refactor/extract-shift-handler-426
Open

refactor: extract ShiftHandler from GeneralKeyboardIME (Part 13) - #426#688
prince-0408 wants to merge 1 commit into
scribe-org:mainfrom
prince-0408:refactor/extract-shift-handler-426

Conversation

@prince-0408

Copy link
Copy Markdown
Collaborator

Description

This PR is Part 13 in modularizing GeneralKeyboardIME for #426.

It extracts shift key state machine management (SHIFT_OFFSHIFT_ON_ONE_CHARSHIFT_ON_PERMANENT), caps lock double-tap timing logic (lastShiftPressTS, shiftPermToggleSpeed), and keyboard mode switching (handleKeyboardLetters, handleModeChange) out of GeneralKeyboardIME.kt into a standalone helper class ShiftHandler.

Detailed Changes Table

File / Component Changes Applied Detailed Impact
ShiftHandler.kt Created standalone helper encapsulating timing state (lastShiftPressTS, shiftPermToggleSpeed), shift state cycling (handleKeyboardLetters), and layout mode switching between letter keyboard and symbol keyboard pages (handleModeChange). Extracts shift key state machine management and keyboard layout mode transitions out of GeneralKeyboardIME.kt into a dedicated helper class.
GeneralKeyboardIME.kt Instantiated shiftHandler and delegated shift and mode switching methods (handleKeyboardLetters, handleModeChange). Simplified KEYCODE_SHIFT key handling in event listener and updated getPrimarySymbolKeyboardLayoutXML() to internal visibility. Fulfills the core goal of #426 by decoupling shift state and layout mode switching from GeneralKeyboardIME.kt while maintaining 100% backward compatibility for all callers.
ShiftHandlerTest.kt Added unit tests covering shift timing state (lastShiftPressTS) and double-tap caps lock lock timing logic. Ensures unit test coverage for shift state helper logic.
CHANGELOG.md Added entry under ### ♻️ Code Refactoring detailing the extraction of ShiftHandler from GeneralKeyboardIME. Satisfies ci_changelog_check workflow requirement for pull requests targeting main.

Key Benefits

  • Decoupled Shift & Layout Switching Logic: Removes shift state machine toggling, double-tap caps lock timing calculations, and letter/symbol keyboard mode switching from GeneralKeyboardIME.kt.
  • Deduplicated Event Listener Logic: Eliminates duplicate shift handling code inside the key press event listener in GeneralKeyboardIME.kt.
  • Improved Maintainability: Shift timing and keyboard page mode switching can now be tested and maintained independently of IME service lifecycles.

Related Issue

Refactors part of #426

@Roniscend

Copy link
Copy Markdown
Collaborator

@prince-0408 Does ShiftHandler accept an injectable time source for the double-tap logic, or is it calling System.currentTimeMillis() directly? Injecting a clock would keep those timing tests deterministic.

@prince-0408

Copy link
Copy Markdown
Collaborator Author

@prince-0408 Does ShiftHandler accept an injectable time source for the double-tap logic, or is it calling System.currentTimeMillis() directly? Injecting a clock would keep those timing tests deterministic.

@Roniscend

At present ShiftHandler directly uses System.currentTimeMillis().

Providing the possibility of injecting timeProvider (for example private val timeProvider: () -> Long = { System.currentTimeMillis() }) via constructor is a great idea. This way, it becomes possible to inject a time provider for the purpose of testing the difference between single tap and double tap lock shift without relying on actual delay times.

I will change ShiftHandler to accept timeProvider in its constructor and create deterministic unit tests for double tap time in PR #688.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants