refactor: extract ConjugationHandler from GeneralKeyboardIME (Part 12) - #426 by prince-0408 · Pull Request #687 · scribe-org/Scribe-Android

prince-0408 · 2026-08-25T09:05:45Z

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

File / Component	Changes Applied	Detailed Impact
`ConjugationHandler.kt`	Created standalone helper encapsulating conjugation state (`subsequentAreaRequired`, `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`).	Extracts verb conjugation table formatting, multi-option selection sub-views, and layout grid mode calculations out of `GeneralKeyboardIME.kt` into a dedicated helper class.
`GeneralKeyboardIME.kt`	Instantiated `conjugationHandler` and 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.	Fulfills the core goal of #426 by decoupling verb conjugation layout and state management from `GeneralKeyboardIME.kt` while maintaining 100% backward compatibility for all callers.
`ConjugationHandlerTest.kt`	Added unit tests covering capitalization formatting (`applyCapitalizationToConjugations`), index boundary clamping (`getValidatedConjugateIndex`), layout mode saving (`saveConjugateModeType`), and XML layout selection (`getKeyboardLayoutForState`).	Ensures unit test coverage for verb conjugation helper logic using Robolectric and MockK.
`CHANGELOG.md`	Added entry under `### ♻️ Code Refactoring` detailing the extraction of `ConjugationHandler` from `GeneralKeyboardIME`.	Satisfies `ci_changelog_check` workflow requirement for pull requests targeting `main`.

Key Benefits

Decoupled Conjugation & Layout Logic: Removes complex grid layout calculations (2x1, 1x3, 3x2, 2x2), verb tense capitalization rules, and multi-option selection sub-views from GeneralKeyboardIME.kt.
Improved Testability & Maintainability: Verb conjugation index validation, mode persistence, layout XML selection, and text capitalization can now be tested and maintained independently of IME service lifecycles.

Related Issue

Refactors part of #426

andrewtavis · 2026-09-05T10:17:07Z

Would you be able to fix the merge conflicts, @prince-0408? We'd then get to the review :)

andrewtavis · 2026-09-13T15:13:23Z

Thanks for the rebase, @prince-0408! We'll focus on bringing this one in next :)

Roniscend · 2026-09-14T09:41:58Z

+    var subsequentAreaRequired: Boolean = false
+    var subsequentData: MutableList<List<String>> = mutableListOf()
+
+    var conjugateOutput: MutableMap<String, MutableMap<String, Collection<String>>>?


This only passes through to ime.conjugateOutput, so read it from ime directly and drop the property.

Roniscend · 2026-09-14T09:42:13Z

+            ime.conjugateOutput = value
+        }
+
+    var conjugateLabels: Set<String>


ConjugateLabels is never used in the handler, so it can be deleted.

Roniscend · 2026-09-14T09:42:31Z

+    ) {
+        val mode =
+            if (isSubsequent) {
+                "2x1"


Sub-view now saves "2x1" here, but it used to save "none". Please keep the original behavior.

Roniscend · 2026-09-14T09:43:01Z

+            if (isSubsequent) {
+                "2x1"
+            } else {
+                when (getLanguageAlias(language).lowercase()) {


SaveConjugateModeType("none") now resolves to English and saves 2x2, so the idle keyboard gets conjugation key heights

Roniscend · 2026-09-14T09:43:37Z

+                "2x1"
+            } else {
+                when (getLanguageAlias(language).lowercase()) {
+                    "es", "it" -> "3x2"


3x2 has no branch in KeyboardBase, so Spanish and Italian fall through to 3x3 height when they used to get 2x2.

Roniscend · 2026-09-14T09:51:57Z

+            conjugationHandler.subsequentAreaRequired = value
+        }
+
+    internal var subsequentData: MutableList<List<String>>


Keep it private or remove the wrapper

Roniscend · 2026-09-14T09:52:32Z

+     * Delegated to [ConjugationHandler].
     */
-    private fun getKeyboardLayoutForState(
+    internal fun getKeyboardLayoutForState(


This was private and is now internal ,does anything outside the class use it?

Roniscend · 2026-09-14T09:53:19Z

+    }
+
+    @Test
+    fun saveConjugateModeType_spanishReturns3x2() {


Please add a test for saveConjugateModeType(none) that expects none

Roniscend · 2026-09-14T09:53:47Z

+
+        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) }


This asserts the LEFT/RIGHT bug, so please update it once line 164 is fixed.

Roniscend · 2026-09-14T09:54:08Z

+    }
+
+    @Test
+    fun returnSubsequentData_and_conjugateLabels() {


This only checks that values are passed through,please test the behavior instead

@Roniscend, big question for the initial review is whether this has been resolved. Feel free to resolve the thread if so :)

…) - scribe-org#426

andrewtavis · 2026-09-17T23:08:09Z

Looks like there are a few minor edits from @Roniscend's review here, @prince-0408 😊 Let us know if you need any support finalizing them 😊

prince-0408 · 2026-09-19T10:45:40Z

Looks like there are a few minor edits from @Roniscend's review here, @prince-0408 😊 Let us know if you need any support finalizing them 😊

Hi @andrewtavis

Everything is up to date and ready for review! 😊

andrewtavis · 2026-09-19T14:31:02Z

CC @Roniscend that an initial review here would be appreciated :)

Roniscend · 2026-09-21T01:08:01Z

+        val flattenList = filteredData.flatten()
+        saveConjugateModeType(language = ime.language, isSubsequent = true)
+        val keyboardXmlId = getKeyboardLayoutForState(ime.currentState, isSubsequentArea = true, dataSize = flattenList.size)
+        ime.uiManager.initializeKeyboard(keyboardXmlId)


Conjugate_mode_type is none when the sub-view inflates here, but on main the removed saveConjugateModeType inside getKeyboardLayoutForState left it 2x2, was the row-height change intended?

Roniscend · 2026-09-21T01:09:09Z

+                when (language) {
+                    "English", "Russian", "Swedish",
+                    "German", "French", "Italian", "Portuguese", "Spanish",
+                    "en", "ru", "sv", "de", "fr", "it", "pt", "es",


Ime.language is scribeLanguage.displayName , so these ISO-code branches are unreachable.

Roniscend · 2026-09-21T01:09:39Z

+            if (!isSubsequent) {
+                when (language) {
+                    "English", "Russian", "Swedish",
+                    "German", "French", "Italian", "Portuguese", "Spanish",


This hardcoded list duplicates defaultConjugateModeType, which each IME already overrides , expose that on KeyboardIMEContext and read it instead.

Roniscend · 2026-09-21T01:10:17Z

+     *
+     * @return A valid, zero-based index for the conjugation type.
+     */
+    fun getValidatedConjugateIndex(): Int {


This is byte-identical to the private getValidatedConjugateIndex already in KeyboardUIManager, keep one and delete the other

Roniscend · 2026-09-21T01:10:47Z

+    private val ime: KeyboardIMEContext,
+) {
+    var subsequentAreaRequired: Boolean = false
+    var subsequentData: MutableList<List<String>> = mutableListOf()


SubsequentData is never assigned anywhere in the codebase, so returnSubsequentData() always returns empty wire it up or drop it

Roniscend · 2026-09-21T01:14:17Z

+
+        val prefs = context.getSharedPreferences("keyboard_preferences", Context.MODE_PRIVATE)
+        assertEquals("none", prefs.getString("conjugate_mode_type", null))
+        verify { uiManager.initializeKeyboard(R.xml.conjugate_view_2x1) }


UiManager is a real KeyboardUIManager , not a mockk, so MockK verifies the relaxed listener calls made inside initializeKeyboard instead and this passes even with the wrong XML id.

Roniscend · 2026-09-21T01:14:46Z

+
+        val prefs = context.getSharedPreferences("keyboard_preferences", Context.MODE_PRIVATE)
+        assertEquals("none", prefs.getString("conjugate_mode_type", null))
+        verify { uiManager.initializeKeyboard(R.xml.conjugate_view_1x3) }


Same here ,mock uiManager or assert on the inflated KeyboardBase so the layout id is actually checked

Roniscend · 2026-09-21T01:15:18Z

+
+    @Test
+    fun saveConjugateModeType_noneReturnsNone() {
+        handler.saveConjugateModeType("none", isSubsequent = false)


None isn't a language the IME can pass , use a genuinely unsupported one like Hindi

Roniscend · 2026-09-21T01:15:43Z

+        every { ime.getInputConnection() } returns inputConnection
+        every { ime.suggestionHandler } returns suggestionHandler
+        every { ime.uiManager } returns uiManager
+        every { ime.binding } returns binding


Every { ime.binding } returns binding is unused , the handler reads ime.uiManager.binding.

Roniscend · 2026-09-21T01:16:03Z

+        assertEquals(View.GONE, binding.ivInfo.visibility)
+        assertFalse(handler.subsequentAreaRequired)
+    }
+}


No test covers the dataSize == 0 path returning defaultConjugateLayoutXML, which is the only branch using the new interface member.

prince-0408 requested a review from andrewtavis August 25, 2026 09:12

prince-0408 mentioned this pull request Aug 25, 2026

Code improvements to make Scribe ready for future features #426

Open

9 tasks

angrezichatterbox added the refactor Refactor code to improve quality label Aug 30, 2026

prince-0408 force-pushed the refactor/extract-conjugation-handler-426 branch from 1cd39ae to d2543e8 Compare September 5, 2026 18:39

andrewtavis added the no-changelog No changelog entry is needed for this pull request label Sep 13, 2026

andrewtavis mentioned this pull request Sep 13, 2026

refactor: extract SuggestionUIHandler from GeneralKeyboardIME (Part 11) (#426) #685

Merged

prince-0408 force-pushed the refactor/extract-conjugation-handler-426 branch 2 times, most recently from f2a4f9e to 71c7ea3 Compare September 13, 2026 14:23

andrewtavis requested review from Roniscend and angrezichatterbox September 13, 2026 15:12

Roniscend suggested changes Sep 14, 2026

View reviewed changes

prince-0408 force-pushed the refactor/extract-conjugation-handler-426 branch 2 times, most recently from 6d344f3 to 2af3627 Compare September 14, 2026 10:35

refactor: extract ConjugationHandler from GeneralKeyboardIME (Part 12…

949c234

…) - scribe-org#426

prince-0408 force-pushed the refactor/extract-conjugation-handler-426 branch from 2af3627 to 949c234 Compare September 14, 2026 10:41

prince-0408 requested a review from Roniscend September 14, 2026 11:02

Merge branch 'main' into refactor/extract-conjugation-handler-426

7e14741

Roniscend suggested changes Sep 21, 2026

View reviewed changes

Conversation

prince-0408 commented Aug 25, 2026

Description

Detailed Changes Table

Key Benefits

Related Issue

Uh oh!

andrewtavis commented Sep 5, 2026

Uh oh!

andrewtavis commented Sep 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewtavis commented Sep 17, 2026

Uh oh!

prince-0408 commented Sep 19, 2026

Uh oh!

andrewtavis commented Sep 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants