refactor: extract ConjugationHandler from GeneralKeyboardIME (Part 12) - #426 - #687
Conversation
|
Would you be able to fix the merge conflicts, @prince-0408? We'd then get to the review :) |
1cd39ae to
d2543e8
Compare
andrewtavis
added
the
no-changelog
f2a4f9e to
71c7ea3
Compare
|
Thanks for the rebase, @prince-0408! We'll focus on bringing this one in next :) |
| var subsequentAreaRequired: Boolean = false | ||
| var subsequentData: MutableList<List<String>> = mutableListOf() | ||
|
|
||
| var conjugateOutput: MutableMap<String, MutableMap<String, Collection<String>>>? |
There was a problem hiding this comment.
This only passes through to ime.conjugateOutput, so read it from ime directly and drop the property.
| ime.conjugateOutput = value | ||
| } | ||
|
|
||
| var conjugateLabels: Set<String> |
There was a problem hiding this comment.
ConjugateLabels is never used in the handler, so it can be deleted.
| ) { | ||
| val mode = | ||
| if (isSubsequent) { | ||
| "2x1" |
There was a problem hiding this comment.
Sub-view now saves "2x1" here, but it used to save "none". Please keep the original behavior.
| if (isSubsequent) { | ||
| "2x1" | ||
| } else { | ||
| when (getLanguageAlias(language).lowercase()) { |
There was a problem hiding this comment.
SaveConjugateModeType("none") now resolves to English and saves 2x2, so the idle keyboard gets conjugation key heights
| "2x1" | ||
| } else { | ||
| when (getLanguageAlias(language).lowercase()) { | ||
| "es", "it" -> "3x2" |
There was a problem hiding this comment.
3x2 has no branch in KeyboardBase, so Spanish and Italian fall through to 3x3 height when they used to get 2x2.
| conjugationHandler.subsequentAreaRequired = value | ||
| } | ||
|
|
||
| internal var subsequentData: MutableList<List<String>> |
There was a problem hiding this comment.
Keep it private or remove the wrapper
| * Delegated to [ConjugationHandler]. | ||
| */ | ||
| private fun getKeyboardLayoutForState( | ||
| internal fun getKeyboardLayoutForState( |
There was a problem hiding this comment.
This was private and is now internal ,does anything outside the class use it?
| } | ||
|
|
||
| @Test | ||
| fun saveConjugateModeType_spanishReturns3x2() { |
There was a problem hiding this comment.
Please add a test for saveConjugateModeType(none) that expects none
|
|
||
| val prefs = context.getSharedPreferences("keyboard_preferences", Context.MODE_PRIVATE) | ||
| assertEquals("2x1", prefs.getString("conjugate_mode_type", null)) | ||
| verify { keyboardView.setKeyLabel("hablo", "HI", KeyboardBase.CODE_1X3_RIGHT) } |
There was a problem hiding this comment.
This asserts the LEFT/RIGHT bug, so please update it once line 164 is fixed.
| } | ||
|
|
||
| @Test | ||
| fun returnSubsequentData_and_conjugateLabels() { |
There was a problem hiding this comment.
This only checks that values are passed through,please test the behavior instead
6d344f3 to
2af3627
Compare
2af3627 to
949c234
Compare
4 participants
Description
This PR is Part 12 in modularizing GeneralKeyboardIME for #426.
It extracts verb conjugation state management, conjugation table layout calculation (2x1, 1x3, 3x2, 2x2), capitalization formatting, index boundary validation, and sub-view setup out of GeneralKeyboardIME.kt into a standalone helper class ConjugationHandler.
Detailed Changes Table
ConjugationHandler.ktsubsequentAreaRequired,subsequentData,conjugateOutput,conjugateLabels), layout mode saving (saveConjugateModeType), capitalization formatting (applyCapitalizationToConjugations), index validation (getValidatedConjugateIndex), conjugation key press handling (handleConjugateKeys), secondary sub-view setup (setupConjugateSubView), and layout XML selection (getKeyboardLayoutForState).GeneralKeyboardIME.ktinto a dedicated helper class.GeneralKeyboardIME.ktconjugationHandlerand delegated conjugation state properties (subsequentAreaRequired,subsequentData) and helper methods (saveConjugateModeType,applyCapitalizationToConjugations,getValidatedConjugateIndex,returnIsSubsequentRequired,returnSubsequentData,handleConjugateKeys,setupConjugateSubView,getKeyboardLayoutForState). Removed duplicate private helper implementations and unused constants.GeneralKeyboardIME.ktwhile maintaining 100% backward compatibility for all callers.ConjugationHandlerTest.ktapplyCapitalizationToConjugations), index boundary clamping (getValidatedConjugateIndex), layout mode saving (saveConjugateModeType), and XML layout selection (getKeyboardLayoutForState).CHANGELOG.md### ♻️ Code Refactoringdetailing the extraction ofConjugationHandlerfromGeneralKeyboardIME.ci_changelog_checkworkflow requirement for pull requests targetingmain.Key Benefits
2x1,1x3,3x2,2x2), verb tense capitalization rules, and multi-option selection sub-views fromGeneralKeyboardIME.kt.Related Issue
Refactors part of #426