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.

…rt 12) - scribe-org#426

Roniscend · 2026-09-21T10:18:40Z

    // Language-specific layout and behavior configurations (decoupled from base class).
-    open val defaultConjugateModeType: String = "3x2"
-    open val defaultConjugateLayoutXML: Int = R.xml.conjugate_view_3x2
+    open override val defaultConjugateModeType: String = "3x2"


German, French, Italian, Portuguese and Spanish don't override this, so they go from 2x2 to 3x2, which KeyboardBase has no case for their conjugation keys silently fall to the 73dp 3x3 height instead of 113dp

Roniscend · 2026-09-21T10:20:40Z

        } else {
            val word = ime.handleConjugateKeys(code, true)
-            ime.setupConjugateSubView(ime.returnSubsequentData(), word)
+            ime.setupConjugateSubView(emptyList(), word)


Hardcoding emptyList() here guarantees setupConjugateSubView always hits its else -> return, permanently disabling the sub-view path instead of keeping the returnSubsequentData() seam

…rt 13) - scribe-org#426

Roniscend · 2026-09-21T12:40:50Z

@andrewtavis Requesting a final review on this pr lgtm from my side

andrewtavis

praise: From my end this is looking great :) Thanks for the collaboration, @prince-0408 and @Roniscend! We'll see how all of this goes once the verb data is back up and running, which hopefully will be soon. Back to Scribe-Data now 😊

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

prince-0408 added 2 commits September 21, 2026 12:58

refactor: address Roniscend review comments on ConjugationHandler (Pa…

e415ea2

…rt 12) - scribe-org#426

fix(ci): fix ktlint consecutive blank lines and sub-view test assertion

3b0c025

prince-0408 requested a review from Roniscend September 21, 2026 09:05

Roniscend suggested changes Sep 21, 2026

View reviewed changes

refactor: address Roniscend review comments on ConjugationHandler (Pa…

443a1f2

…rt 13) - scribe-org#426

prince-0408 requested a review from Roniscend September 21, 2026 12:10

Roniscend approved these changes Sep 21, 2026

View reviewed changes

andrewtavis approved these changes Sep 21, 2026

View reviewed changes

andrewtavis merged commit 6758fac into scribe-org:main Sep 21, 2026
7 checks passed

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Roniscend commented Sep 21, 2026

Uh oh!

andrewtavis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants