refactor: extract KeyboardLayoutHandler from GeneralKeyboardIME (Part 15) - #426 - #692
Conversation
|
Great that this is open, @prince-0408! Would this be the last PR to close the issue? |
Thanks @andrewtavis! Not the last one -- Part 16 (introducing a KeyboardIMEContext interface contract as suggested by @Roniscend in #689) is already raised. That should be the final PR to close #426. |
| } else { | ||
| when (dataSize) { | ||
| DATA_SIZE_2 -> R.xml.conjugate_view_2x1 | ||
| DATA_CONSTANT_3 -> R.xml.conjugate_view_1x3 |
There was a problem hiding this comment.
Naming nit , DATA_CONSTANT_3 should be DATA_SIZE_3 to match DATA_SIZE_2 wherever data_constant is mentioned
There was a problem hiding this comment.
Updated DATA_CONSTANT_3 to DATA_SIZE_3 for consistency.
| import android.text.InputType.TYPE_MASK_CLASS | ||
| import be.scri.R | ||
| import be.scri.models.ScribeState | ||
| import be.scri.services.GeneralKeyboardIME |
There was a problem hiding this comment.
Circular import: KeyboardLayoutHandler (helpers) and GeneralKeyboardIME (services) import each other, the handler isn't truly decoupled, it just holds a full ime reference.
There was a problem hiding this comment.
Good catch. In this PR (Part 15), KeyboardLayoutHandler followed the same structure as previous helper extractions. In the dedicated follow-up #693 (Part 16), we introduce the KeyboardIMEContext interface contract across all handlers, removing the GeneralKeyboardIME reference from KeyboardLayoutHandler and breaking the circular import across the codebase.
| TYPE_CLASS_NUMBER, TYPE_CLASS_DATETIME, TYPE_CLASS_PHONE -> true | ||
| else -> false | ||
| } | ||
| internal fun shouldUseNumericKeyboard(inputType: Int): Boolean = KeyboardLayoutHandler.shouldUseNumericKeyboard(inputType) |
There was a problem hiding this comment.
Wrappers still hang around shouldUseNumericKeyboard and getKeyboardLayoutXMLForInputType in the companion object just forward to KeyboardLayoutHandler now ,could remove them and call KeyboardLayoutHandler directly if nothing else still uses these.
There was a problem hiding this comment.
Removed the forwarding wrappers from GeneralKeyboardIME companion object and updated all call sites and unit tests to invoke KeyboardLayoutHandler directly.
|
@prince-0408: Would you be able to rebase and resolve conflicts? Anything else needed after that, or would you say that we're ready for a final review? :) |
b222e92 to
250fe75
Compare
250fe75 to
2ab5b6a
Compare
3 participants
Description
This PR is Part 15 in modularizing GeneralKeyboardIME for #426.
It extracts keyboard layout XML resolution (
getCurrentKeyboardLayoutXML,getPrimarySymbolKeyboardLayoutXML,getKeyboardLayoutForState), container width calculation (getKeyboardWidth), input-type keyboard layout matching (getKeyboardLayoutXMLForInputType,shouldUseNumericKeyboard), and keyboard view re-creation (recreateKeyboard) out ofGeneralKeyboardIME.ktinto a standalone helper classKeyboardLayoutHandler.Detailed Changes Table
KeyboardLayoutHandler.ktgetCurrentKeyboardLayoutXML,getPrimarySymbolKeyboardLayoutXML,getKeyboardLayoutForState), container width calculation (getKeyboardWidth), input-type layout matching (getKeyboardLayoutXMLForInputType,shouldUseNumericKeyboard), and view re-creation (recreateKeyboard).GeneralKeyboardIME.ktlayoutHandlerdelegate and delegated layout methods (getCurrentKeyboardLayoutXML,getKeyboardWidth,recreateKeyboard,getKeyboardLayoutForState). Delegated companion input type layout helpers (getKeyboardLayoutXMLForInputType,shouldUseNumericKeyboard). Removed dead extensioncoerceInSafe.GeneralKeyboardIME.ktwhile preserving 100% backward compatibility.KeyboardLayoutHandlerTest.ktCHANGELOG.md### ♻️ Code Refactoringdetailing the extraction ofKeyboardLayoutHandlerfromGeneralKeyboardIME.ci_changelog_checkworkflow requirement for pull requests targetingmain.Key Benefits
GeneralKeyboardIME.kt.Related Issue
Refactors part of #426