fix: 한손 모아키 ㅣ/ㅡ 위로 스와이프 입력 수정 및 관련 버그 수정#8
Merged
Conversation
- OpenMoaView: moeumKey 위쪽 제스처(keyList[0]) 값을 ᆢ → ㆍ로 수정 - CrossKeyTouchListener: resolveKeyFromGesture 순수 함수 분리, require(size==4) 가드 추가 - CrossKeyTouchListenerTest: 방향별 구별 가능한 testKeyList 도입 및 경계값 테스트 추가
- dimTextColor() 추가: HSV 명도를 88%로 낮춰 키 텍스트보다 약간 어둡게 표시 - setSuggestions/showSelectionActions에서 색상을 한 번만 계산해 각 뷰에 전달 (이전: buildWordView/buildActionView 호출마다 SharedPreferences 재조회) - applyColors() 루프에서 기존 TextView 업데이트 시 dimmed currentTextColor 사용 - applyColors() 자체도 currentTextColor에 dimTextColor 적용값 저장하도록 수정
- LandscapeKoLayout enum 신설 (NONE/QWERTY/QWERTY_SIMPLE) - 기존 boolean landscape_qwerty 설정을 enum으로 마이그레이션 - resolveKoLayout()에서 단모음 여부 전달하도록 수정 - 설정 화면: SwitchPreference → ListPreference (3-옵션) - 설정 타이틀: '가로 모드 입력 방식', NONE 라벨: '세로와 동일(기본)' - '쿼티' 표현 제거
buildWordView에서 word.trim()으로 표시해 자동치환 expansion의 trailing space가 시각적 간격 차이로 보이던 문제 해결. 입력 콜백엔 원본 word(trailing space 포함)를 유지해 자동치환 동작 보존.
📝 WalkthroughWalkthroughThis PR updates the landscape Korean input layout preference from a boolean to an enum-backed system with backward-compatible migration, refactors gesture direction detection into a reusable helper, fixes the moeum key character, and refactors suggestion bar color rendering to use skin-based styling. ChangesLandscape Korean Layout Migration
Gesture Resolution Refactoring and Moeum Key Fix
Suggestion Bar Color Rendering Refactoring
Version Update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/kotlin/dev/bsb/moakeyvim/settings/SettingsFragment.kt (1)
423-431:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd
KEY_LANDSCAPE_QWERTYtoBOOLEAN_KEYSset to preventClassCastExceptionduring import.The import logic at lines 423-431 stores untyped keys via
optString(), falling through the else block for any key not inBOOLEAN_KEYSorINT_KEYS. SinceKEY_LANDSCAPE_QWERTYis inALLOWED_KEYSbut missing fromBOOLEAN_KEYS, imported JSON with"landscape_qwerty": true/falsegets stored as a String. Later,SettingsPreferences.getBoolean(KEY_LANDSCAPE_QWERTY, false)will throwClassCastExceptionwhen attempting to read the String value as a boolean.Suggested fix
private val BOOLEAN_KEYS = setOf( SettingsPreferences.KEY_KEY_PREVIEW, SettingsPreferences.KEY_AUTO_SPACE_PERIOD, SettingsPreferences.KEY_AUTO_CAPITALIZE_ENGLISH, SettingsPreferences.KEY_HOTSTRING_ENABLED, SettingsPreferences.KEY_WORD_SUGGESTION_ENABLED, SettingsPreferences.KEY_KOREAN_WORD_SUGGESTION_ENABLED, SettingsPreferences.KEY_CLIPBOARD_ENABLED, SettingsPreferences.KEY_FLOATING_INDICATOR_ENABLED, SettingsPreferences.KEY_OVERLAY_PERMISSION_NOTIFIED, SettingsPreferences.KEY_HW_CAPSLOCK_TO_CTRL, SettingsPreferences.KEY_HW_TAB_VIM_MODE, + SettingsPreferences.KEY_LANDSCAPE_QWERTY, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/kotlin/dev/bsb/moakeyvim/settings/SettingsFragment.kt` around lines 423 - 431, The import code treats keys not in BOOLEAN_KEYS or INT_KEYS as strings, causing a ClassCastException for KEY_LANDSCAPE_QWERTY; update the BOOLEAN_KEYS set to include KEY_LANDSCAPE_QWERTY so the when branch uses settingsObj.optBoolean(key) and stores a Boolean in settingsEdits (ensure you modify the BOOLEAN_KEYS declaration that contains the set of boolean keys and not ALLOWED_KEYS or INT_KEYS).
🧹 Nitpick comments (1)
app/src/main/kotlin/dev/bsb/moakeyvim/view/suggestion/SuggestionBarView.kt (1)
125-129: 💤 Low valueConsider extracting the repeated skin-color pattern.
The pattern of fetching the skin, getting the foreground color, and dimming it appears in both
setSuggestions()(lines 126-127) andshowSelectionActions()(lines 178-179). While this works correctly, extracting it to a helper method could improve maintainability.♻️ Optional refactor
Add a helper method:
private fun getDimmedForegroundColor(): Int { val skin = SettingsPreferences.getKeyboardSkin(context) return dimTextColor(SkinApplier.fgColor(context, skin)) }Then use it in both places:
- val skin = SettingsPreferences.getKeyboardSkin(context) - val wordColor = dimTextColor(SkinApplier.fgColor(context, skin)) + val wordColor = getDimmedForegroundColor() words.forEach { word -> container.addView(buildWordView(word, word in hotstringExpansions, wordColor)) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/kotlin/dev/bsb/moakeyvim/view/suggestion/SuggestionBarView.kt` around lines 125 - 129, Extract the repeated skin/color logic used in setSuggestions() and showSelectionActions() into a small helper method (e.g., getDimmedForegroundColor()) that calls SettingsPreferences.getKeyboardSkin(context), resolves SkinApplier.fgColor(context, skin) and returns dimTextColor(...) so both setSuggestions() and showSelectionActions() call this helper instead of duplicating the three-step sequence; update calls to use getDimmedForegroundColor() and keep method visibility private inside SuggestionBarView.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/kotlin/dev/bsb/moakeyvim/settings/SettingsFragment.kt`:
- Around line 127-128: The current call to
pref<ListPreference>(SettingsPreferences.KEY_LANDSCAPE_KO_LAYOUT)?.setupEnum(...,
LandscapeKoLayout.NONE) forces a default of NONE and can overwrite legacy
landscape_qwerty before migration; change it to preserve any existing/migrated
value by removing the hard-default (do not pass LandscapeKoLayout.NONE) and
instead pass null or the current stored value retrieved from SettingsPreferences
(or run migration first) so setupEnum uses the existing preference; update the
call site (pref<ListPreference>(SettingsPreferences.KEY_LANDSCAPE_KO_LAYOUT) and
setupEnum) to avoid forcing NONE.
In
`@app/src/test/kotlin/dev/bsb/moakeyvim/view/keytouchlistener/CrossKeyTouchListenerTest.kt`:
- Line 71: The test in CrossKeyTouchListenerTest.kt uses Kotlin's
assert(result.key != "ᆢ") which is ignored without -ea; replace it with a JUnit
assertion such as assertNotEquals("ᆢ", result.key) (or assertTrue(result.key !=
"ᆢ")) to ensure the check always runs; update the assertion call in the test
method (referencing result.key) using the existing JUnit imports.
---
Outside diff comments:
In `@app/src/main/kotlin/dev/bsb/moakeyvim/settings/SettingsFragment.kt`:
- Around line 423-431: The import code treats keys not in BOOLEAN_KEYS or
INT_KEYS as strings, causing a ClassCastException for KEY_LANDSCAPE_QWERTY;
update the BOOLEAN_KEYS set to include KEY_LANDSCAPE_QWERTY so the when branch
uses settingsObj.optBoolean(key) and stores a Boolean in settingsEdits (ensure
you modify the BOOLEAN_KEYS declaration that contains the set of boolean keys
and not ALLOWED_KEYS or INT_KEYS).
---
Nitpick comments:
In `@app/src/main/kotlin/dev/bsb/moakeyvim/view/suggestion/SuggestionBarView.kt`:
- Around line 125-129: Extract the repeated skin/color logic used in
setSuggestions() and showSelectionActions() into a small helper method (e.g.,
getDimmedForegroundColor()) that calls
SettingsPreferences.getKeyboardSkin(context), resolves
SkinApplier.fgColor(context, skin) and returns dimTextColor(...) so both
setSuggestions() and showSelectionActions() call this helper instead of
duplicating the three-step sequence; update calls to use
getDimmedForegroundColor() and keep method visibility private inside
SuggestionBarView.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9954cdb9-1e5a-4003-9f74-cf065cdb5a9c
📒 Files selected for processing (11)
app/build.gradleapp/src/main/kotlin/dev/bsb/moakeyvim/OpenMoaIME.ktapp/src/main/kotlin/dev/bsb/moakeyvim/config/LandscapeKoLayout.ktapp/src/main/kotlin/dev/bsb/moakeyvim/settings/SettingsFragment.ktapp/src/main/kotlin/dev/bsb/moakeyvim/settings/SettingsPreferences.ktapp/src/main/kotlin/dev/bsb/moakeyvim/view/keyboardview/OpenMoaView.ktapp/src/main/kotlin/dev/bsb/moakeyvim/view/keytouchlistener/CrossKeyTouchListener.ktapp/src/main/kotlin/dev/bsb/moakeyvim/view/suggestion/SuggestionBarView.ktapp/src/main/res/values/strings.xmlapp/src/main/res/xml/preferences_main.xmlapp/src/test/kotlin/dev/bsb/moakeyvim/view/keytouchlistener/CrossKeyTouchListenerTest.kt
Comment on lines
+127
to
+128
| pref<ListPreference>(SettingsPreferences.KEY_LANDSCAPE_KO_LAYOUT) | ||
| ?.setupEnum(LandscapeKoLayout.values(), { it.labelResId }, LandscapeKoLayout.NONE) |
There was a problem hiding this comment.
Preserve migrated legacy value instead of hard-defaulting to NONE.
Line 127~128 currently initializes landscape_ko_layout to LandscapeKoLayout.NONE when unset, which can overwrite users’ legacy landscape_qwerty=true before migration logic is applied.
Suggested fix
- pref<ListPreference>(SettingsPreferences.KEY_LANDSCAPE_KO_LAYOUT)
- ?.setupEnum(LandscapeKoLayout.values(), { it.labelResId }, LandscapeKoLayout.NONE)
+ pref<ListPreference>(SettingsPreferences.KEY_LANDSCAPE_KO_LAYOUT)
+ ?.setupEnum(
+ LandscapeKoLayout.values(),
+ { it.labelResId },
+ SettingsPreferences.getLandscapeKoLayout(requireContext())
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pref<ListPreference>(SettingsPreferences.KEY_LANDSCAPE_KO_LAYOUT) | |
| ?.setupEnum(LandscapeKoLayout.values(), { it.labelResId }, LandscapeKoLayout.NONE) | |
| pref<ListPreference>(SettingsPreferences.KEY_LANDSCAPE_KO_LAYOUT) | |
| ?.setupEnum( | |
| LandscapeKoLayout.values(), | |
| { it.labelResId }, | |
| SettingsPreferences.getLandscapeKoLayout(requireContext()) | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/kotlin/dev/bsb/moakeyvim/settings/SettingsFragment.kt` around
lines 127 - 128, The current call to
pref<ListPreference>(SettingsPreferences.KEY_LANDSCAPE_KO_LAYOUT)?.setupEnum(...,
LandscapeKoLayout.NONE) forces a default of NONE and can overwrite legacy
landscape_qwerty before migration; change it to preserve any existing/migrated
value by removing the hard-default (do not pass LandscapeKoLayout.NONE) and
instead pass null or the current stored value retrieved from SettingsPreferences
(or run migration first) so setupEnum uses the existing preference; update the
call site (pref<ListPreference>(SettingsPreferences.KEY_LANDSCAPE_KO_LAYOUT) and
setupEnum) to avoid forcing NONE.
| fun `모아키 모음키 위로 스와이프는 아래아 단일 반환 - 이중 아래아 아님`() { | ||
| val result = resolveKeyFromGesture(0f, 100f, 0f, 0f, threshold, moeumKeyList()) | ||
| assertEquals("ㆍ", result.key) | ||
| assert(result.key != "ᆢ") { "위로 스와이프에 아래아 이중(ᆢ)이 반환되면 안 됨" } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether JVM assertions are enabled for tests and find Kotlin assert usage in test sources.
rg -n --iglob '*gradle*' 'jvmArgs|enableAssertions|-ea'
rg -n --iglob '*Test.kt' '\bassert\s*\('Repository: verlane/moakey-vim
Length of output: 866
🏁 Script executed:
#!/bin/bash
# Find all gradle configuration files
find . -name "build.gradle*" -o -name "settings.gradle*" | head -20
# Check for gradle wrapper properties that might affect JVM args
cat gradle/wrapper/gradle-wrapper.properties 2>/dev/null || echo "No gradle wrapper properties found"
# Search more broadly for test task configuration and JVM argument settings
rg -U 'test\s*\{|jvmArgs|java\s*\{' --type kotlin --type gradle -A 5 | head -50Repository: verlane/moakey-vim
Length of output: 365
🏁 Script executed:
#!/bin/bash
# Check the main build.gradle and app/build.gradle for test configuration
echo "=== ./build.gradle ==="
cat ./build.gradle
echo -e "\n=== ./app/build.gradle ==="
cat ./app/build.gradle
# Also check if JUnit is in dependencies
echo -e "\n=== Checking for JUnit in dependencies ==="
rg 'junit|jUnit' build.gradle app/build.gradleRepository: verlane/moakey-vim
Length of output: 2009
🏁 Script executed:
#!/bin/bash
# Check the test file to understand context and verify assert usage
cat -n app/src/test/kotlin/dev/bsb/moakeyvim/view/keytouchlistener/CrossKeyTouchListenerTest.kt | sed -n '65,80p'
# Check what JUnit imports are used in the test
head -30 app/src/test/kotlin/dev/bsb/moakeyvim/view/keytouchlistener/CrossKeyTouchListenerTest.ktRepository: verlane/moakey-vim
Length of output: 1169
Replace Kotlin assert with JUnit assertion.
Line 71 uses Kotlin's assert(...) which does not execute unless JVM is started with -ea flag. The gradle configuration has no such flag enabled, so this check will silently pass without validation. Use JUnit's assertTrue(result.key != "ᆢ") or assertNotEquals("ᆢ", result.key) instead, which are already imported in the file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/src/test/kotlin/dev/bsb/moakeyvim/view/keytouchlistener/CrossKeyTouchListenerTest.kt`
at line 71, The test in CrossKeyTouchListenerTest.kt uses Kotlin's
assert(result.key != "ᆢ") which is ignored without -ea; replace it with a JUnit
assertion such as assertNotEquals("ᆢ", result.key) (or assertTrue(result.key !=
"ᆢ")) to ensure the check always runs; update the assertion call in the test
method (referencing result.key) using the existing JUnit imports.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
한손 모아키에서 모음키(ㅣ/ㅡ)를 위로 스와이프할 때 이중 아래아(ᆢ)가 입력되던 버그를 수정하고, 가로 모드 두벌식 단모음 입력 방식 설정 및 추천단어 바 UI 개선을 함께 포함합니다.
Closes #2
Changes
Bug Fixes
CrossKeyTouchListener)Features
LandscapeKoLayout,SettingsPreferences)Files Changed
CrossKeyTouchListener.ktCrossKeyTouchListenerTest.ktLandscapeKoLayout.ktSettingsPreferences.ktSettingsFragment.ktSuggestionBarView.ktOpenMoaView.ktOpenMoaIME.ktstrings.xmlpreferences_main.xmlapp/build.gradleTesting
CrossKeyTouchListenerTest단위 테스트 추가 및 통과 확인Related Issues
Closes #2
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes