Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
Walkthrough여러 레이아웃의 EditText에 하이라이트 색상과 EditText 전용 테마 속성을 추가하고, 해당 테마 스타일( Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/res/values/styles.xml`:
- Around line 20-22: The Theme.Websoso.EditText style only changes
colorControlActivated but not the actual cursor drawable, so override
android:textCursorDrawable in the Theme.Websoso.EditText style to point to the
desired color/drawable (e.g., same `@color/primary_100_6A5DFD` or a tintable
drawable) so the text cursor color matches the selection; reference the
Theme.Websoso.EditText style and the android:textCursorDrawable attribute (and
ensure it overrides the Base.Theme.Websoso setting).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7e494595-a681-4bb7-a35c-0fc0a7c2105a
📒 Files selected for processing (4)
app/src/main/res/layout/activity_create_feed.xmlapp/src/main/res/layout/activity_feed_detail.xmlapp/src/main/res/layout/activity_normal_explore.xmlapp/src/main/res/values/styles.xml
| <style name="Theme.Websoso.EditText" parent=""> | ||
| <item name="colorControlActivated">@color/primary_100_6A5DFD</item> | ||
| </style> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 기준 테마와 EditText 오버레이가 어떤 커서 관련 속성을 선언하는지 확인합니다.
rg -n -C2 'Theme\.Websoso\.EditText|textCursorDrawable|colorControlActivated' \
app/src/main/res/values/styles.xml \
app/src/main/res/values/themes.xmlRepository: Team-WSS/WSS-Android
Length of output: 1016
커서 색상을 실제로 바꾸려면 android:textCursorDrawable도 함께 오버라이드해야 합니다.
Base.Theme.Websoso가 themes.xml Line 7에서 android:textCursorDrawable을 @color/black으로 설정하고 있으나, 현재 스타일은 colorControlActivated만 변경하고 있습니다. 이로 인해 선택 하이라이트만 변경되고 실제 텍스트 커서는 검은색으로 유지됩니다.
수정 예시
<style name="Theme.Websoso.EditText" parent="">
<item name="colorControlActivated">@color/primary_100_6A5DFD</item>
+ <item name="android:textCursorDrawable">@color/primary_100_6A5DFD</item>
</style>📝 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.
| <style name="Theme.Websoso.EditText" parent=""> | |
| <item name="colorControlActivated">@color/primary_100_6A5DFD</item> | |
| </style> | |
| <style name="Theme.Websoso.EditText" parent=""> | |
| <item name="colorControlActivated">@color/primary_100_6A5DFD</item> | |
| <item name="android:textCursorDrawable">@color/primary_100_6A5DFD</item> | |
| </style> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/res/values/styles.xml` around lines 20 - 22, The
Theme.Websoso.EditText style only changes colorControlActivated but not the
actual cursor drawable, so override android:textCursorDrawable in the
Theme.Websoso.EditText style to point to the desired color/drawable (e.g., same
`@color/primary_100_6A5DFD` or a tintable drawable) so the text cursor color
matches the selection; reference the Theme.Websoso.EditText style and the
android:textCursorDrawable attribute (and ensure it overrides the
Base.Theme.Websoso setting).
There was a problem hiding this comment.
토끼 리뷰처럼 textCursorDrawable를 사용하지 않은 이유도 궁금합니다!
There was a problem hiding this comment.
@devfeijoa, 좋은 질문입니다! textCursorDrawable을 사용하지 않은 이유와 두 방식의 차이를 정리하면 다음과 같습니다.
colorControlActivated vs android:textCursorDrawable
colorControlActivated는 포커스 상태일 때의 커서 색상을 포함하여, 부동 레이블 및 밑줄 색상을 제어하는 테마 속성입니다.
이 방식이 동작하는 이유는 colorControlActivated가 특정 컴포넌트 속성(예: hintTextColor)이 아닌 테마 속성이기 때문입니다. 따라서 android:theme을 통한 테마 오버레이로 적용할 수 있습니다.
반면 android:textCursorDrawable은 직접 뷰 속성으로, 커서 드로어블을 명시적으로 지정합니다. colorControlActivated는 themed context 내 모든 활성화된 컨트롤에 영향을 주는 반면, cursorColor 같은 전용 속성은 커서 드로어블만 정밀하게 타겟합니다. 격리된 커서 변경에는 전용 속성이, 전체 통일된 강조색에는 colorControlActivated가 적합합니다.
제 리뷰 코멘트가 제기한 우려
themes.xml Line 7에서 Base.Theme.Websoso가 android:textCursorDrawable="@color/black"을 명시적으로 설정하고 있습니다. 이 경우, colorControlActivated만 테마 오버레이로 변경하면 색상 틴팅이 적용될 수 있지만, 명시적으로 지정된 black 드로어블이 우선할 가능성이 있습니다.
PR 스크린샷 확인
PR description 이미지를 보면 검색 바에 "fkdkdk"가 입력되어 있고, 커서가 보라/파란색으로 표시되어 있습니다. 이는 colorControlActivated 방식이 이 앱의 특정 구현에서 실제로 동작함을 시각적으로 보여주고 있습니다.
결론
작성자가 textCursorDrawable을 사용하지 않은 이유는 아마도:
- Material Components에서 권장하는 테마 오버레이 방식 (
colorControlActivated)으로 커서 색상 변경이 가능함을 확인했기 때문 textCursorDrawable을 스타일에 추가하려면 별도의 drawable XML 파일(또는 API 29+ 색상 직접 참조)이 필요해 더 복잡해지기 때문
다만, Base.Theme.Websoso가 명시적으로 android:textCursorDrawable을 설정하고 있으므로, Android의 커서 렌더링 메커니즘이 API 레벨마다 다르게 동작하기 때문에 다양한 기기에서 동작을 검증하는 것을 권장합니다. 스크린샷만으로는 모든 기기/API 레벨에서의 동작이 보장되지 않습니다.
| android:textColor="@color/black" | ||
| android:textColorHint="@color/gray_200_949399" | ||
| android:textColorHighlight="@color/primary_50_F1EFFF" | ||
| android:theme="@style/Theme.Websoso.EditText" |
There was a problem hiding this comment.
해당 부분 보다가 궁금한 점이 있어서요!
기존에는 색상 리소스를 colors에 정의해서 사용하는 경우가 많았던 것 같은데, 이번에는 styles 쪽에서colorControlActivated로 연결해주셨더라고요.
혹시 이렇게 스타일로 분리해서 적용하신 이유나 기준이 있을까요?
저도 이후 작업할 때 같은 기준으로 맞추고 싶어서 여쭤봅니다 :)
There was a problem hiding this comment.
선택 손잡이(handle) 부분을 colorControlActivated(?)로 구현한게 맞는지? 이걸로 구현하신 이유가 뭔지도 궁금합니다!
There was a problem hiding this comment.
compose는 MaterialTheme에 속성값을 추가하여 모든 TextField에 전체적용을 할 수 있는데, xml은 일일이 모든 editText에 속성값을 넣어야 하는걸로 알고 있습니다.
그리고 안드로이드 시스템에서 물방울 손잡이? 와 커서 선 색상은 xml에서 직접적인 속성값이 없고 colorControlActivated 이걸 따라가는 걸로 알고있습니다. 그래서 theme를 만들어 edittext에 적용했습니다.
There was a problem hiding this comment.
textCursorDrawable 이건 속성값을 지정하는 거라 theme로 설정하면 theme값을 따라가는 것으로 알고있습니다!
| <style name="Theme.Websoso.EditText" parent=""> | ||
| <item name="colorControlActivated">@color/primary_100_6A5DFD</item> | ||
| </style> |
There was a problem hiding this comment.
토끼 리뷰처럼 textCursorDrawable를 사용하지 않은 이유도 궁금합니다!
2 participants
📌𝘐𝘴𝘴𝘶𝘦𝘴
📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯
📷𝘚𝘤𝘳𝘦𝘦𝘯𝘴𝘩𝘰𝘵
💬𝘛𝘰 𝘙𝘦𝘷𝘪𝘦𝘸𝘦𝘳𝘴
UI가 xml 형태로 되어있어 전체 적용이 불가능합니다. 추후 compose로 변경되면 MaterialTheme에서 적용하면 될듯 합니다.
Summary by CodeRabbit
변경 사항