Conversation
as-is:innerTextField()가 말줄임 텍스트에 대체되어, 재포커스 시 커서 위치가 맨 앞으로 초기화됨 to-be:innerTextField()는 항시 유지하고 말줄임 텍스트 컴포저블을 스택으로 쌓아 커서 위치 유지
📝 WalkthroughWalkthroughCompose 기반 필드 컴포넌트 세트(베이식, 쇼트/롱/카운트, 드롭다운, 서치)와 관련 모델·유틸이 추가되었고, 플레이스홀더 파일 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PotiSearchField
participant PotiBasicField
participant PotiDropdownMenu
participant PotiMenuItem
participant Keyboard
User->>PotiSearchField: 클릭/포커스
PotiSearchField->>PotiBasicField: 포커스 요청 / 렌더
PotiSearchField->>PotiDropdownMenu: expanded = true
PotiDropdownMenu->>PotiDropdownMenu: 애니메이션 표시
User->>PotiBasicField: 텍스트 입력
PotiBasicField->>PotiSearchField: onValueChange(text)
PotiSearchField->>PotiDropdownMenu: 필터/아이템 업데이트
User->>PotiMenuItem: 항목 클릭
PotiMenuItem->>PotiSearchField: onItemClick(item)
PotiSearchField->>PotiDropdownMenu: expanded = false (조건부)
PotiDropdownMenu->>PotiDropdownMenu: 애니메이션 숨김
User->>Keyboard: Done 누름
Keyboard->>PotiSearchField: onSearch/onDone
PotiSearchField->>PotiBasicField: 포커스 해제 / 키보드 숨김
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 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
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: 5
🤖 Fix all issues with AI agents
In
@app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt:
- Around line 31-49: Rename the misspelled parameter onValueChaged to
onValueChanged in PotiBasicField and update all internal usages to match (also
update the five call sites that pass the named parameter). Replace the default
focusRequester: FocusRequester = FocusRequester() signature with focusRequester:
FocusRequester? = null and inside PotiBasicField create val actualFocusRequester
= focusRequester ?: remember { FocusRequester() } then use actualFocusRequester
for focusRequester usage (e.g., requesting focus or passing to
Modifier.focusRequester) to avoid recreating a FocusRequester on recomposition.
In
@app/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.kt:
- Line 106: PotiCountField currently ignores its enabled parameter by passing a
hardcoded true into PotiBasicField; update the PotiCountField implementation to
forward the received enabled argument to PotiBasicField (replace the literal
true with the enabled parameter) so callers can disable the field; verify the
parameter name enabled used in PotiCountField matches the one passed into
PotiBasicField.
In
@app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.kt:
- Around line 119-129: The icon logic is inverted: inside the Crossfade
observing expandedState.targetState (in PotiDropdownField's Crossfade block)
swap the drawable selection so closed shows ic_arrow_down_lg and opened shows
ic_arrow_up_lg; update the ImageVector.vectorResource conditional that currently
uses (if (opened) R.drawable.ic_arrow_down_lg else R.drawable.ic_arrow_up_lg) to
the correct one (if (opened) R.drawable.ic_arrow_up_lg else
R.drawable.ic_arrow_down_lg) so the Icon reflects down when closed and up when
open.
In
@app/src/main/java/com/poti/android/core/designsystem/component/field/PotiMenuItem.kt:
- Around line 18-65: The PotiMenuItem currently still responds to clicks and
shows the pressed background when disabled; update the modifier.clickable call
to disable interaction by passing enabled = !disabled and keep using the same
interactionSource, and change the pressed background logic in the
backgroundColor calculation to only apply when isPressed && !disabled (e.g.,
when determining backgroundColor use "isPressed && !disabled ->
PotiTheme.colors.gray100" else white) so disabled items neither respond to
clicks nor show the pressed state; adjust only the clickable(...) parameters and
the backgroundColor when-expression in the PotiMenuItem composable.
In @app/src/main/java/com/poti/android/core/designsystem/model/FieldMenuItem.kt:
- Around line 5-10: FieldMenuItem currently uses a default-random id (id: String
= UUID.randomUUID().toString()) which can change across recompositions; change
the API so callers must provide a stable id (remove the default value for the id
parameter on data class FieldMenuItem) or add a separate factory/helper (e.g.,
FieldMenuItemFactory.create...) that generates IDs once outside UI composition;
update call sites to supply stable business-derived IDs or use the factory to
avoid instability in dropdown/search keys and selection.
🧹 Nitpick comments (15)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt (1)
78-110: Overlay-Text(value)ellipsis path: consider clipping + semantics to avoid double-announcement.Because
innerTextField()is still composed and you drawText(value)over it (Line 89 + Line 99+), consider:
- adding
Modifier.clipToBounds()on theBox(or theBasicTextFieldmodifier) to prevent any potential draw overflow from the underlying text, and- marking the overlay
Text(value)as non-semantic (orclearAndSetSemantics { }) if TalkBack ends up reading both.app/src/main/java/com/poti/android/core/designsystem/component/field/FieldLabel.kt (1)
8-17: Optional: acceptmodifierand considerisNotBlank()for labels.This is small, but adding
modifier: Modifier = Modifiermakes the label easier to place/test, andlabel.isNotBlank()avoids rendering whitespace-only labels.app/src/main/java/com/poti/android/core/designsystem/component/field/FieldErrorMessage.kt (1)
10-28: Decide whether this is a stable public API (and track TODO as an issue), and simplify layout if possible.
- The TODO (Line 10) suggests this will be removed after a merge; if so, consider making it
internalnow (or link an issue/target version) to avoid exposing a short-lived public API.- If no icon is planned, the
Row(Line 17) is likely redundant;Text(modifier = modifier, ...)would be simpler.app/src/main/java/com/poti/android/core/designsystem/component/field/PotiLongTextField.kt (2)
92-105: Preview function naming inconsistency.The preview function names reference "ShortTextField" but this file implements
PotiLongTextField. Also, "Preveiw" is misspelled.✏️ Suggested fix
@Preview @Composable -private fun PotiShortTextFieldWithErrorPreveiw() { +private fun PotiLongTextFieldWithErrorPreview() { var text by remember { mutableStateOf("") } PotiTheme { PotiLongTextField( value = text, onValueChanged = { text = it }, placeholder = "플레이스홀더", error = "에러 메시지", ) } }
107-120: Preview function naming inconsistency.Same issue as above - should be
PotiLongTextFieldWithLabelPreview.✏️ Suggested fix
@Preview @Composable -private fun PotiShortTextFieldWithLabelPreveiw() { +private fun PotiLongTextFieldWithLabelPreview() { var text by remember { mutableStateOf("") }app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownMenu.kt (2)
42-42: Typo in parameter name:popupProterties.Should be
popupPropertiesfor consistency and readability.✏️ Suggested fix
- popupProterties: PopupProperties = PopupProperties(), + popupProperties: PopupProperties = PopupProperties(),Also update the usage on line 67:
- properties = popupProterties, + properties = popupProperties,
47-62: Unusedoffset.xin position calculation.The
offsetparameter includes both x and y components, but onlyoffset.yis used in the position calculation. The x-offset is ignored and the popup always aligns toanchorBounds.left. This could be confusing for callers.Consider either using
offset.xor changing the parameter type to justDpfor the y-offset to better reflect actual behavior.♻️ Option 1: Use both offset components
val popupPositionProvider = remember(offset, density) { + val offsetXPx = with(density) { offset.x.roundToPx() } val offsetYPx = with(density) { offset.y.roundToPx() } object : PopupPositionProvider { override fun calculatePosition( anchorBounds: IntRect, windowSize: IntSize, layoutDirection: LayoutDirection, popupContentSize: IntSize, ): IntOffset { return IntOffset( - x = anchorBounds.left, + x = anchorBounds.left + offsetXPx, y = anchorBounds.bottom + offsetYPx, ) } } }app/src/main/java/com/poti/android/core/designsystem/component/field/PotiShortTextField.kt (1)
97-125: Preview function names contain typo."Preveiw" should be "Preview" in both preview function names.
✏️ Suggested fix
@Preview @Composable -private fun PotiShortTextFieldWithErrorPreveiw() { +private fun PotiShortTextFieldWithErrorPreview() {@Preview @Composable -private fun PotiShortTextFieldWithLabelPreveiw() { +private fun PotiShortTextFieldWithLabelPreview() {app/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.kt (1)
114-144: Preview function names contain typo."Preveiw" should be "Preview" in both preview function names.
✏️ Suggested fix
@Preview @Composable -private fun PotiCountFieldWithErrorPreveiw() { +private fun PotiCountFieldWithErrorPreview() {@Preview @Composable -private fun PotiCountFieldWithLabelPreveiw() { +private fun PotiCountFieldWithLabelPreview() {app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.kt (3)
117-117: Inconsistent color reference.Uses
Whitedirectly instead ofPotiTheme.colors.whitewhich is used in other field components (e.g.,PotiShortTextField,PotiLongTextField,PotiCountField).✏️ Suggested fix
- backgroundColor = White, + backgroundColor = PotiTheme.colors.white,
175-207: Preview function name contains typo and excessive line length."Preveiw" should be "Preview". Also, line 181 is excessively long and hard to read.
✏️ Suggested fix for naming
@Preview @Composable -private fun PotiDropdownFieldPreveiw() { +private fun PotiDropdownFieldPreview() {Consider breaking the long
menuItemslist into multiple lines for readability.
209-247: Preview function name contains typo."Preveiw" should be "Preview".
✏️ Suggested fix
@Preview @Composable -private fun PotiDropdownFieldWithPriceWithMutlipleSelectPreveiw() { +private fun PotiDropdownFieldWithPriceWithMultipleSelectPreview() {Note: "Mutliple" is also misspelled - should be "Multiple".
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt (3)
52-52: Typo in KDoc comment."없ㅇ르" should be "없을".
✏️ Suggested fix
- * @param placeholder 입력값 및 선택한 옵션이 없ㅇ르 때 필드에 표시됩니다. + * @param placeholder 입력값 및 선택한 옵션이 없을 때 필드에 표시됩니다.
103-122: Complex state management with magic delays.The state management using multiple boolean flags (
searchActionDone,dismissRequestDone,isTyping) with hardcodeddelay(100)calls is difficult to follow and may lead to race conditions or unexpected behavior.Consider:
- Extracting the delay value to a named constant with documentation explaining its purpose
- Consolidating the state flags into a sealed class or enum representing distinct states
private const val DEBOUNCE_DELAY_MS = 100L // Delay to prevent flickering during rapid state changes
146-146: Inconsistent color reference.Uses
Whitedirectly instead ofPotiTheme.colors.white, which is inconsistent with other field components.✏️ Suggested fix
- backgroundColor = White, + backgroundColor = PotiTheme.colors.white,
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
app/src/main/java/com/poti/android/core/designsystem/component/field/DummyFieldComponent.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/FieldErrorMessage.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/FieldLabel.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownMenu.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiLongTextField.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiMenuItem.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiShortTextField.ktapp/src/main/java/com/poti/android/core/designsystem/model/FieldMenuItem.kt
💤 Files with no reviewable changes (1)
- app/src/main/java/com/poti/android/core/designsystem/component/field/DummyFieldComponent.kt
🧰 Additional context used
🧬 Code graph analysis (5)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.kt (4)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt (1)
PotiBasicField(31-115)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownMenu.kt (1)
PotiDropdownMenu(32-80)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiMenuItem.kt (1)
PotiMenuItem(18-65)app/src/main/java/com/poti/android/core/designsystem/theme/Theme.kt (1)
PotiTheme(48-71)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt (4)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt (1)
PotiBasicField(31-115)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownMenu.kt (1)
PotiDropdownMenu(32-80)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiMenuItem.kt (1)
PotiMenuItem(18-65)app/src/main/java/com/poti/android/core/designsystem/theme/Theme.kt (1)
PotiTheme(48-71)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.kt (4)
app/src/main/java/com/poti/android/core/designsystem/component/field/FieldLabel.kt (1)
FieldLabel(7-18)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt (1)
PotiBasicField(31-115)app/src/main/java/com/poti/android/core/designsystem/component/field/FieldErrorMessage.kt (1)
FieldErrorMessage(11-28)app/src/main/java/com/poti/android/core/designsystem/theme/Theme.kt (1)
PotiTheme(48-71)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiLongTextField.kt (4)
app/src/main/java/com/poti/android/core/designsystem/component/field/FieldLabel.kt (1)
FieldLabel(7-18)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt (1)
PotiBasicField(31-115)app/src/main/java/com/poti/android/core/designsystem/component/field/FieldErrorMessage.kt (1)
FieldErrorMessage(11-28)app/src/main/java/com/poti/android/core/designsystem/theme/Theme.kt (1)
PotiTheme(48-71)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiShortTextField.kt (4)
app/src/main/java/com/poti/android/core/designsystem/component/field/FieldLabel.kt (1)
FieldLabel(7-18)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt (1)
PotiBasicField(31-115)app/src/main/java/com/poti/android/core/designsystem/component/field/FieldErrorMessage.kt (1)
FieldErrorMessage(11-28)app/src/main/java/com/poti/android/core/designsystem/theme/Theme.kt (1)
PotiTheme(48-71)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR CI Check (Lint & Build)
🔇 Additional comments (7)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiLongTextField.kt (1)
35-90: LGTM!The
PotiLongTextFieldimplementation is well-structured with proper focus management, border color states, and keyboard action handling. TheheightIn(160.dp)correctly allows dynamic height growth for multiline content.app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownMenu.kt (1)
82-125: LGTM!The
PotiDropdownMenuContentproperly handles animation transitions and conditional max height constraints.app/src/main/java/com/poti/android/core/designsystem/component/field/PotiShortTextField.kt (1)
38-95: LGTM!The
PotiShortTextFieldimplementation is well-structured and consistent with other field components. Proper focus management and keyboard action handling are in place.app/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.kt (1)
45-112: LGTM on overall structure.The character count display in the trailing icon is a nice touch. The documentation clearly states that
maxLengthis for UI display only and character limiting should be handled externally, which is a reasonable design choice.app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.kt (1)
63-173: LGTM on overall structure.The dropdown field implementation properly handles state management for expand/collapse, positioning, and item selection. Good use of
Crossfadefor smooth icon transitions andonGloballyPositionedfor dynamic width calculation.app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt (2)
203-242: Preview test logic doesn't match PR description.The preview uses
"hih"as the test string, but the PR description mentions testing with "안녕" input yielding "안녕하세요" search results. Consider updating the preview to match the documented behavior or clarify the discrepancy.
65-201: LGTM on overall functionality.The search field properly integrates with
PotiBasicFieldandPotiDropdownMenu, handling keyboard search actions and item selection appropriately.
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiMenuItem.kt
Show resolved
Hide resolved
| data class FieldMenuItem( | ||
| val option: String, | ||
| val price: String? = null, | ||
| val disabled: Boolean = false, | ||
| val id: String = UUID.randomUUID().toString(), | ||
| ) |
There was a problem hiding this comment.
Avoid default-random id if items are created in/near recomposition.
id: String = UUID.randomUUID().toString() (Line 9) can change across rebuilds if callers instantiate items without passing id, which can cause key/selection instability in dropdown/search lists. Prefer requiring id from callers, deriving it from stable business data, or providing a separate factory that generates IDs once (outside UI composition).
🤖 Prompt for AI Agents
In @app/src/main/java/com/poti/android/core/designsystem/model/FieldMenuItem.kt
around lines 5 - 10, FieldMenuItem currently uses a default-random id (id:
String = UUID.randomUUID().toString()) which can change across recompositions;
change the API so callers must provide a stable id (remove the default value for
the id parameter on data class FieldMenuItem) or add a separate factory/helper
(e.g., FieldMenuItemFactory.create...) that generates IDs once outside UI
composition; update call sites to supply stable business-derived IDs or use the
factory to avoid instability in dropdown/search keys and selection.
as-is : 메뉴 오픈 시 보더 컬러 변경 to-be : 필드 포커스 시 보더 컬러 변경
|
✅ Ktlint check passed. Run: https://github.com/team-poti/POTI-ANDROID/actions/runs/20956326174 |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In
@app/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.kt:
- Line 116: Rename the two misspelled preview functions
(PotiCountFieldWithErrorPreveiw and the other PotiCountFieldPreveiw) to use the
correct "Preview" spelling (PotiCountFieldWithErrorPreview and
PotiCountFieldPreview), update any references or usages and ensure any @Preview
annotations remain on the renamed functions so the previews compile and show
correctly.
- Line 43: Fix the typo in the KDoc @sample tag for PotiCountField: change the
sample reference name from PotiCountFieldWithLabelPreveiw to
PotiCountFieldWithLabelPreview (update the @sample line in the KDoc above the
PotiCountField declaration so the sample identifier matches the actual
composable preview function name).
- Line 79: PotiBasicField declares a misspelled parameter name onValueChaged;
rename it to onValueChanged in PotiBasicField (both declaration and usages
inside that file, e.g., the function/class where the parameter is defined and
referenced), then update all callers to use the corrected name: PotiCountField
(replace onValueChaged → onValueChanged at the invocation), PotiLongTextField,
PotiShortTextField, PotiSearchField, and PotiDropdownField; ensure function
signatures and named-argument calls match the new onValueChanged identifier and
run the build to catch any remaining references.
In
@app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt:
- Line 54: Fix the KDoc typo in PotiSearchField's documentation: change "없ㅇ르" to
"없을" in the @param placeholder description so the comment reads correctly ("입력값
및 선택한 옵션이 없을 때 필드에 표시됩니다.")—update the KDoc for the placeholder parameter in
PotiSearchField.kt accordingly.
- Around line 221-224: The enum SearchType is missing a trailing comma which
causes a lint failure; update the enum declaration (SearchType with entries
ARTIST and PRODUCT) to include a trailing comma after the last entry
(PRODUCT(156.dp),) so the list ends with a comma before the closing brace.
- Around line 262-264: The call to the PotiSearchField composable is missing a
trailing comma before the closing parenthesis which causes a lint/pipeline
failure; update the invocation (the block setting searchType =
SearchType.ARTIST, error = "에러 메시지") to include a trailing comma after the last
argument (e.g., error = "에러 메시지",) so the formatter/linter accepts the call in
PotiSearchField.kt.
- Around line 140-142: The Column declaration is missing a trailing comma which
breaks linting; update the Column call that currently reads
Column(verticalArrangement = Arrangement.spacedBy(8.dp)) { to include a trailing
comma after the verticalArrangement argument (i.e., add a comma after
Arrangement.spacedBy(8.dp)) so the multiline Compose call is properly
terminated.
- Line 82: The default parameter uses remember { FocusRequester() } which
recreates on recomposition; change the parameter signature for PotiSearchField
to accept a nullable FocusRequester? with a default of null (focusRequester:
FocusRequester? = null), then inside the function create an
actualFocusRequester: val actualFocusRequester = focusRequester ?: remember {
FocusRequester() } and replace any uses of focusRequester with
actualFocusRequester so the remembered FocusRequester is created only once per
composition when the caller doesn't supply one.
🧹 Nitpick comments (2)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt (2)
109-128: Consider documenting or extracting magic delay values.The
delay(100)calls are unexplained magic numbers. Consider extracting them to a named constant with documentation explaining why 100ms is chosen, or add inline comments explaining the purpose of these delays.Suggested improvement
private const val DROPDOWN_STATE_DEBOUNCE_MS = 100L // Debounce to prevent flickering during rapid state changes // Then use: delay(DROPDOWN_STATE_DEBOUNCE_MS)
238-243: RedundantmenuItems.clear()call.Line 238 already clears
menuItems, so themenuItems.clear()at line 242 in the else branch is redundant.Proposed fix
onValueChange = { text = it menuItems.clear() if (text == "hih") { menuItems.add(FieldMenuItem("hih")) - } else { - menuItems.clear() } },
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiMenuItem.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/com/poti/android/core/designsystem/component/field/PotiMenuItem.kt
- app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.kt
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt (5)
app/src/main/java/com/poti/android/core/designsystem/component/field/FieldLabel.kt (1)
FieldLabel(7-18)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt (1)
PotiBasicField(31-115)app/src/main/java/com/poti/android/core/designsystem/component/field/FieldErrorMessage.kt (1)
FieldErrorMessage(11-28)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownMenu.kt (1)
PotiDropdownMenu(32-80)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiMenuItem.kt (1)
PotiMenuItem(18-67)
🪛 GitHub Actions: PR CI Check
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt
[error] 141-141: KtLint: Missing trailing comma before ')' (standard: trailing-comma-on-call-site)
[error] 223-223: KtLint: Missing trailing comma before '}' (standard: trailing-comma-on-declaration-site)
[error] 263-263: KtLint: Missing trailing comma before ')' (standard: trailing-comma-on-call-site)
🔇 Additional comments (1)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.kt (1)
45-112: Well-structured component with clear separation of concerns.The implementation follows good Compose practices:
- Proper state hoisting with
value/onValueChangedpattern.- Clean focus and keyboard handling.
- Sensible default parameters.
- Comprehensive KDoc documentation.
The character count display logic and border color prioritization (error > focused > default) are implemented correctly.
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
@app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.kt:
- Line 65: The @sample tag and preview function names contain a typo: change
every occurrence of PotiDropdownFieldPreveiw to PotiDropdownFieldPreview; update
the @sample line that references PotiDropdownFieldPreveiw and rename the preview
function declaration(s) (the preview function at/around the original line 195
and any other occurrences e.g., around line 229) to PotiDropdownFieldPreview,
and update any calls/annotations referencing the old misspelled identifier so
all declarations and references match the corrected name.
- Around line 119-123: The onFocusChanged callback attached to the modifier (the
block using onFocusChanged and checking focusState.isFocused to set
expandedState.targetState) is ineffective because PotiBasicField is created with
enabled = false and disabled composables can't gain focus; remove that
onFocusChanged modifier block entirely and rely on the existing clickable
handler, item selection logic, and Popup.onDismissRequest to control
expandedState instead (update any modifier chaining around onFocusChanged
accordingly and remove any now-unused focus-related imports).
In
@app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt:
- Line 54: Fix the typo in the KDoc for the PotiSearchField component: update
the @param placeholder description in PotiSearchField.kt to replace "없ㅇ르" with
the correct "없을" so the comment reads "...입력값 및 선택한 옵션이 없을 때 필드에 표시됩니다.".
🧹 Nitpick comments (2)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt (1)
236-243: Simplify preview logic and align with documentation.The preview's test logic has two issues:
Redundant
elsebranch: Line 238 already clearsmenuItems, so theelseblock on line 241-243 that clears it again is unnecessary.Documentation mismatch: The PR description mentions testing with "안녕" → "안녕하세요", but the preview uses "hih" → "hih". Consider aligning the preview with the documented example for clarity.
♻️ Proposed simplification
onValueChange = { text = it menuItems.clear() if (text == "안녕") { - menuItems.add(FieldMenuItem("hih")) - } else { - menuItems.clear() + menuItems.add(FieldMenuItem("안녕하세요")) } },app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.kt (1)
198-200: Simplify menu item list creation in preview.The long list of 30 identical
FieldMenuItem("옵션")items on line 199 is verbose and hard to maintain.♻️ Proposed simplification
val menuItems = listOf( - FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), FieldMenuItem("옵션"), - ) + val menuItems = List(30) { FieldMenuItem("옵션") }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiMenuItem.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt
🧰 Additional context used
🧬 Code graph analysis (2)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt (5)
app/src/main/java/com/poti/android/core/designsystem/component/field/FieldLabel.kt (1)
FieldLabel(7-18)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt (1)
PotiBasicField(31-115)app/src/main/java/com/poti/android/core/designsystem/component/field/FieldErrorMessage.kt (1)
FieldErrorMessage(11-28)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownMenu.kt (1)
PotiDropdownMenu(32-80)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiMenuItem.kt (1)
PotiMenuItem(18-67)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.kt (6)
app/src/main/java/com/poti/android/core/designsystem/component/field/FieldLabel.kt (1)
FieldLabel(7-18)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt (1)
PotiBasicField(31-115)app/src/main/java/com/poti/android/core/designsystem/component/field/FieldErrorMessage.kt (1)
FieldErrorMessage(11-28)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownMenu.kt (1)
PotiDropdownMenu(32-80)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiMenuItem.kt (1)
PotiMenuItem(18-67)app/src/main/java/com/poti/android/core/designsystem/theme/Theme.kt (1)
PotiTheme(48-71)
🔇 Additional comments (1)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiMenuItem.kt (1)
18-67: LGTM! Clean menu item implementation with proper state handling.The component correctly handles interaction states (pressed, selected, disabled) with appropriate visual feedback. The use of
indication = nullprovides a custom press effect via background color changes, which aligns with the design requirements.
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt
Show resolved
Hide resolved
sonyerim
left a comment
sonyerim
left a comment
There was a problem hiding this comment.
커스텀 드롭다운 어렵고 힘들었을텐데 진짜 수고했서용!!
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownMenu.kt
Show resolved
Hide resolved
jyvnee
left a comment
jyvnee
left a comment
There was a problem hiding this comment.
완전 수고했다!!!!! 열심히 고민한 흔적이 보여...........
근데 프리뷰 Preveiw라고 오타낸거 많다 ㅋㅎㅋㅎ 프리뷰는 안 고쳐도 되는데 onValueChaged 오타는 고쳐주라!!!!
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.kt
Outdated
Show resolved
Hide resolved
| val expandedState = remember { MutableTransitionState(false) } | ||
| var parentWidth by remember { mutableIntStateOf(0) } | ||
|
|
||
| LaunchedEffect(Unit) { | ||
| if (initialOpenState) { | ||
| expandedState.targetState = true | ||
| } | ||
| } |
There was a problem hiding this comment.
P2: 여기 expandedState에 MutableTransitionState(initialOpenState) 이렇게 바로 초기값 넣어주지 않고 LaunchedEffect 사용한 이유가 있나요??
There was a problem hiding this comment.
initialOpenState가 true일 때, 화면 진입 시 메뉴가 열렸다 바로 닫히는 문제가 있었어요
근데 그 이유가 onFocusChanged 때문이어서 onFocusChanged 삭제하니 본 문제가 해결되어 LaunchedEffect도 지웠어요!!
구체적인 이유는 필드 초기화 시 onFocusChanged 발동 > 필드 enabled = false 설정되어 있어 focusState = false 조건문 통과 > 메뉴 닫는 로직 발동 으로 예상됩니당
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiLongTextField.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiShortTextField.kt
Outdated
Show resolved
Hide resolved
- 아이템 간 디바이더 대신 bottomBorder 적용 - 서치 필드 포커스 로직 제거 - 드롭다운 필드의 메뉴 최대 높이 아이템 높이에 따라 조정 - 메뉴 아이템 패딩 수정 및 최소 높이 설정, 리플 익스텐션 적용 - 드롭다운 메뉴 Column -> LazyColumn 변경
cmj7271
left a comment
cmj7271
left a comment
There was a problem hiding this comment.
코드 엄청 길고, 복잡하네요... 고생하셨습니당
코리 한번만 확인해주세요~
| trailingIcon: @Composable () -> Unit = {}, | ||
| enabled: Boolean = true, | ||
| ) { | ||
| var isFocused by remember { mutableStateOf(false) } |
There was a problem hiding this comment.
p1: 상태를 컴퍼저블 내부에서 가지고 있으면 안될거 같아요.
interactionSource: MutableInteractionSource = remember { MutableInteractionSource() },
를 파라미터로 받고,
val isFocused by interactionSource.collectIsFocusedAsState() 를 사용하는건 어떨까요?
이렇게 하면 부모 컴포저블에서 상태관리도 가능하면서, 기존 기능 유지할 수 있을거 같아요!
만약 적용한다면, 다른 Field 도 동일할 것 같습니다!
There was a problem hiding this comment.
움 왜 안되는지 이유도 함께 적어주심 감사드립니다!
PotiBasicField내isFocused의 역할- singleLine일 때 focus out 시 말줄임 텍스트 노출해달라는 디자인 요구사항을 위함
- 래핑 컴포넌트 (
PotiCountField등) 내 포커스 관리 필요할 시- 필요에 따라
isFocused생성해 사용 (예: borderColor 관리)
- 필요에 따라
PotiBasicField 내 isFocused는 모든 singleLine 컴포넌트에 적용되는 내용이며 자체적으로 가진 포커스 여부에 따라 제어되는 부분이어서, 부모 컴포저블에서 상태관리하지 않아도 괜찮다는 것이 제 생각입니다!
BasicField는 부모 컴포저블이 관리하는 컴포넌트이기 보다는, 최소한의 로직과 기본 레이아웃을 보장하는 컴포넌트로서 이를 래핑해 유연하게 사용하는 목적으로 설계했어요.
| shape: Shape, | ||
| border: BorderStroke, | ||
| maxHeight: Dp?, | ||
| popupProterties: PopupProperties = PopupProperties(), |
There was a problem hiding this comment.
p3: PopupProperties 오타발견 크크
| shape: Shape = RoundedCornerShape(8.dp), | ||
| border: BorderStroke = BorderStroke(1.dp, PotiTheme.colors.gray700), | ||
| ) { | ||
| val expandedState = remember { MutableTransitionState(false) } |
There was a problem hiding this comment.
확장여부의 상태를 내부로 관리하는 이유가 있을까용?(state hoisting 안하고)
There was a problem hiding this comment.
호이스팅하지 않은 이유는 다음과 같아요.
- 외부에서 여닫기를 관리하는 경우는, 초기 open 여부가 유일 (해당 부분은 initialOpenState로 관리 중입니다)
- 현재 유저가 어디를 터치했느냐에 따라 외부에서 메뉴 여닫기를 관리할 이유의 부재
- 외부에서 관리하려면 메뉴를 여닫는 로직을 모두 콜백으로 호이스팅 해야해서 뷰 로직이 복잡해짐
컴포넌트 내에서 들고 있어도 충분한 값이어서, 뷰에서의 사용성을 좋게 하고자 내부에 두었어용
|
|
||
| @Composable | ||
| internal fun PotiDropdownMenu( | ||
| expandedState: MutableTransitionState<Boolean>, |
There was a problem hiding this comment.
https://github.com/team-poti/POTI-ANDROID/pull/14/changes#r2685028916
만약 PotiDropdownField 가 extand 를 Boolean 으로 관리한다면...
expanded: Boolean 으로 받고,
내부적으로 val expandedState = remember { MutableTransitionState(expanded) } 인 형태..?
There was a problem hiding this comment.
죄송하지만 의도를 잘 이해하지 못햇서용..
PotiDropdownField가 expandedState 대신 expanded: Boolean 값을 관리하면 좋겠다는 의미인가요?
PotiDropdownField에서는 유저 액션에 따라 expandedState.target값을 조정합니다.
Expanded 상태 관리는 PotiDropdownField에서, 상태에 따른 애니메이션은 PotiDropdownMenu에서 처리하는 구조여서 그대로 파라미터로 전달해주도록 구현했어요.
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiLongTextField.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiMenuItem.kt
Outdated
Show resolved
Hide resolved
| LaunchedEffect(isFieldFocused, menuItems.size) { | ||
| if (isFieldFocused) { | ||
| delay(100) | ||
| expandedState.targetState = menuItems.isNotEmpty() | ||
| } | ||
| } | ||
|
|
||
| LaunchedEffect(dismissRequestDone) { | ||
| if (dismissRequestDone) { | ||
| delay(100) | ||
| if (searchActionDone || isTyping) { | ||
| searchActionDone = false | ||
| isTyping = false | ||
| } else { | ||
| expandedState.targetState = false |
There was a problem hiding this comment.
p2: delay(100) 을 줘서 flag 로 관리한 이유가 있나요?
아래처럼 합쳐서 LaunchEffect 없이 처리하는건 안되나요?
fun onSearch() {
onSearchClick(value)
expandedState.targetState = false
clearFocusAndHideKeyboard()
}
fun clearFocusAndHideKeyboard() {
focusManager.clearFocus()
keyboardController?.hide()
}There was a problem hiding this comment.
요기에 설명 있습니다~!
다시 설명드리자면 키보드 입력/서치 아이콘 클릭 발생한 경우, UX적으로 메뉴가 유지되어야 하나
팝업의 onDismissRequest가 클릭 이벤트보다 가장 먼저 호출되어 메뉴가 닫혀버립니다.
flag는 유저 사용성 개선을 위함이고 delay는 flag가 의도한 대로 동작하기 위한 방편이에요.
- FieldStatus 통해 borderColor 관리 - focusRequester 기본값 null로 하고, null인 경우에만 PotiBasicField 내에서 객체 생성
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.kt:
- Around line 168-186: The height-accumulation logic in the onGloballyPositioned
block skips adding the current item's height when the sum would exceed maxHeight
and incorrectly leaves calculatedMaxHeight lower than intended; change the block
to compute a potentialHeight = calculatedMaxHeight + heightDp and then if
potentialHeight > maxHeight set calculatedMaxHeight = maxHeight and
isCalculateFinished = true, else set calculatedMaxHeight = potentialHeight
(avoid returning before updating calculatedMaxHeight). Also fix the bottom
border condition from index < menuItems.size to index < menuItems.lastIndex (or
index != menuItems.lastIndex) so the last item does not show a bottom border;
reference symbols: calculatedMaxHeight, maxHeight, isCalculateFinished,
onGloballyPositioned, showBottomBorder, index, menuItems.
In
@app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt:
- Around line 197-211: The showBottomBorder condition is wrong because index <
menuItems.size is always true; update the condition in the itemsIndexed loop
that builds PotiMenuItem inside PotiSearchField so the bottom border is hidden
for the last element (e.g., use index < menuItems.size - 1 or index !=
menuItems.lastIndex). Locate the itemsIndexed(menuItems) { index, item -> ...
PotiMenuItem(... showBottomBorder = ...) } and replace the boolean expression so
showBottomBorder is false for the final item.
🧹 Nitpick comments (3)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.kt (2)
82-84:heightIn명시적 파라미터 사용 권장
heightIn(52.dp)는 최소 높이를 설정하지만, 명시적으로heightIn(min = 52.dp)로 작성하면 의도가 더 명확해집니다.♻️ 제안 수정
modifier = Modifier .fillMaxWidth() - .heightIn(52.dp), + .heightIn(min = 52.dp),
108-109: TODO 확인
FieldErrorMessage를Display>ErrorMessage로 대체 예정임을 확인했습니다. 해당 작업을 추적하기 위해 이슈를 생성하는 것이 좋을 수 있습니다.이 TODO 항목을 추적하기 위한 이슈를 생성해 드릴까요?
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt (1)
112-130:menuItems.size대신menuItems를 키로 사용하는 것을 권장합니다.
menuItems.size를 키로 사용하면 아이템 개수가 같지만 내용이 다른 리스트로 교체될 때 Effect가 재실행되지 않습니다. 또한delay(100)은 매직 넘버이므로 상수로 추출하면 가독성이 향상됩니다.♻️ 권장 수정 사항
- LaunchedEffect(isFieldFocused, menuItems.size) { + LaunchedEffect(isFieldFocused, menuItems) { if (isFieldFocused) { expandedState.targetState = menuItems.isNotEmpty() } } LaunchedEffect(dismissRequestDone) { if (dismissRequestDone) { - delay(100) + delay(DISMISS_DELAY_MS)상수 정의:
private const val DISMISS_DELAY_MS = 100L
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/src/main/java/com/poti/android/core/common/extension/ModifierExt.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownMenu.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiLongTextField.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiMenuItem.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.ktapp/src/main/java/com/poti/android/core/designsystem/component/field/PotiShortTextField.kt
🚧 Files skipped from review as they are similar to previous changes (4)
- app/src/main/java/com/poti/android/core/designsystem/component/field/PotiLongTextField.kt
- app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt
- app/src/main/java/com/poti/android/core/designsystem/component/field/PotiMenuItem.kt
- app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownMenu.kt
🧰 Additional context used
🧬 Code graph analysis (4)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiShortTextField.kt (4)
app/src/main/java/com/poti/android/core/designsystem/component/field/FieldLabel.kt (1)
FieldLabel(7-18)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt (1)
PotiBasicField(31-122)app/src/main/java/com/poti/android/core/designsystem/component/field/FieldErrorMessage.kt (1)
FieldErrorMessage(11-28)app/src/main/java/com/poti/android/core/designsystem/theme/Theme.kt (1)
PotiTheme(48-71)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.kt (5)
app/src/main/java/com/poti/android/core/designsystem/component/field/FieldLabel.kt (1)
FieldLabel(7-18)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt (1)
PotiBasicField(31-122)app/src/main/java/com/poti/android/core/designsystem/component/field/FieldErrorMessage.kt (1)
FieldErrorMessage(11-28)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownMenu.kt (1)
PotiDropdownMenu(31-79)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiMenuItem.kt (1)
PotiMenuItem(21-76)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.kt (4)
app/src/main/java/com/poti/android/core/designsystem/component/field/FieldLabel.kt (1)
FieldLabel(7-18)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt (1)
PotiBasicField(31-122)app/src/main/java/com/poti/android/core/designsystem/component/field/FieldErrorMessage.kt (1)
FieldErrorMessage(11-28)app/src/main/java/com/poti/android/core/designsystem/theme/Theme.kt (1)
PotiTheme(48-71)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt (6)
app/src/main/java/com/poti/android/core/designsystem/component/field/FieldLabel.kt (1)
FieldLabel(7-18)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiBasicField.kt (1)
PotiBasicField(31-122)app/src/main/java/com/poti/android/core/designsystem/component/field/FieldErrorMessage.kt (1)
FieldErrorMessage(11-28)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownMenu.kt (1)
PotiDropdownMenu(31-79)app/src/main/java/com/poti/android/core/designsystem/component/field/PotiMenuItem.kt (1)
PotiMenuItem(21-76)app/src/main/java/com/poti/android/core/designsystem/theme/Theme.kt (1)
PotiTheme(48-71)
🔇 Additional comments (14)
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiCountField.kt (4)
64-68: 상태 우선순위 로직이 적절합니다.에러 상태가 포커스보다 우선순위가 높아 올바른 시각적 피드백을 제공합니다.
97-104: 글자 수 표시 구현이 적절합니다.문서에 명시된 대로
maxLength는 UI 표시용이며, 실제 글자 수 제한은 외부에서 제어합니다.value.length가maxLength를 초과할 수 있다는 점을 호출자가 인지하도록 문서화가 잘 되어 있습니다.
113-143: Preview 함수 구현이 적절합니다.
private접근 제어자로 외부 노출을 방지PotiTheme래퍼로 테마 적용- 에러 상태와 라벨 상태를 각각 시연하여 다양한 사용 사례 확인 가능
64-68: 이 리뷰 코멘트는 코드가 올바르게 작성되었으므로 필요하지 않습니다.
FieldStatus열거형은 동일 패키지의PotiBasicField.kt파일(124-128줄)에 정의되어 있으며,borderColor확장 속성도 같은 파일(130-135줄)에 정의되어 있습니다. 두 파일 모두com.poti.android.core.designsystem.component.field패키지에 있으므로 명시적 import 없이 접근 가능하며, 코드는 정상적으로 작동합니다.Likely an incorrect or invalid review comment.
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiShortTextField.kt (5)
1-21: LGTM!패키지 선언과 임포트가 적절하게 구성되어 있습니다. 모든 임포트가 코드에서 사용되고 있습니다.
23-50: LGTM!KDoc 문서화가 잘 되어 있고, 함수 시그니처가 Compose 컨벤션을 따르고 있습니다. 파라미터 기본값들이 적절하게 설정되어 있습니다.
62-93: LGTM!레이아웃 구조가 깔끔하고 디자인 시스템 패턴을 잘 따르고 있습니다.
FieldLabel과FieldErrorMessage가 내부적으로 빈 문자열을 처리하므로 조건부 렌더링이 필요 없어 코드가 간결합니다.
96-124: LGTM!프리뷰 함수들이 적절하게 구현되어 있습니다.
PotiTheme으로 래핑하여 테마 색상을 사용할 수 있고,remember와mutableStateOf로 인터랙티브한 프리뷰가 가능합니다.
56-60: 코드가 정상적으로 작동합니다.FieldStatus열거형과borderColor확장 프로퍼티는 동일 패키지(com.poti.android.core.designsystem.component.field)의PotiBasicField.kt에서 올바르게 정의되어 있으며,PotiShortTextField.kt에서 정상적으로 접근 및 사용 가능합니다.app/src/main/java/com/poti/android/core/common/extension/ModifierExt.kt (1)
141-160: LGTM!
bottomBorder확장 함수가 깔끔하게 구현되었습니다.isVisible파라미터로 조건부 렌더링이 가능하고, stroke 위치 계산도 정확합니다. 기존topRoundedBorder함수와 일관된 패턴을 따르고 있습니다.app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt (1)
216-219: LGTM!
SearchTypeenum이 깔끔하게 정의되었습니다. 각 타입별 최대 높이를 명확하게 관리할 수 있습니다.app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.kt (3)
65-94: LGTM!컴포넌트 선언과 상태 관리가 잘 구성되어 있습니다. KDoc 문서화가 상세하고, 파라미터별 역할이 명확합니다.
105-137: LGTM!
enabled = false로 텍스트 입력을 비활성화하고clickablemodifier로 드롭다운 토글을 처리하는 패턴이 적절합니다.Crossfade애니메이션으로 화살표 아이콘 전환도 자연스럽습니다.
191-264: Preview 코드가 잘 작성되었습니다.단일 선택과 다중 선택 시나리오를 모두 커버하는 Preview가 포함되어 있어 테스트에 유용합니다.
app/src/main/java/com/poti/android/core/designsystem/component/field/PotiDropdownField.kt
Show resolved
Hide resolved
| itemsIndexed(menuItems) { index, item -> | ||
| PotiMenuItem( | ||
| option = item.option, | ||
| onClick = { | ||
| onItemClick(item) | ||
| expandedState.targetState = false | ||
| clearFocusAndHideKeyboard() | ||
| }, | ||
| isSelected = item.id in selectedIds, | ||
| modifier = Modifier.fillMaxWidth(), | ||
| price = item.price, | ||
| disabled = item.disabled, | ||
| showBottomBorder = index < menuItems.size, | ||
| ) | ||
| } |
There was a problem hiding this comment.
showBottomBorder 조건이 항상 true입니다.
index < menuItems.size는 itemsIndexed에서 제공하는 모든 인덱스(0부터 size-1)에 대해 항상 true입니다. 마지막 아이템의 하단 테두리를 숨기려면 조건을 수정해야 합니다.
🐛 수정 제안
PotiMenuItem(
option = item.option,
onClick = {
onItemClick(item)
expandedState.targetState = false
clearFocusAndHideKeyboard()
},
isSelected = item.id in selectedIds,
modifier = Modifier.fillMaxWidth(),
price = item.price,
disabled = item.disabled,
- showBottomBorder = index < menuItems.size,
+ showBottomBorder = index < menuItems.size - 1,
)🤖 Prompt for AI Agents
In
@app/src/main/java/com/poti/android/core/designsystem/component/field/PotiSearchField.kt
around lines 197 - 211, The showBottomBorder condition is wrong because index <
menuItems.size is always true; update the condition in the itemsIndexed loop
that builds PotiMenuItem inside PotiSearchField so the bottom border is hidden
for the last element (e.g., use index < menuItems.size - 1 or index !=
menuItems.lastIndex). Locate the itemsIndexed(menuItems) { index, item -> ...
PotiMenuItem(... showBottomBorder = ...) } and replace the boolean expression so
showBottomBorder is false for the final item.
4 participants
Related issue 🛠️
Work Description ✏️
Screenshot 📸
Short
default.mp4
*포커스 아웃 시 텍스트 말줄임 되는 기능은 BasicField 내부에서 구현되었습니다.
Long
default.mp4
Count
default.mp4
Dropdown
default.mp4
Search
default.mp4
*임의로 "안녕" 입력 시 "안녕하세요"가 검색결과로 들어오는 로직 넣어 테스트했습니다.
Uncompleted Tasks 😅
To Reviewers 📢
트러블슈팅 보시면 조금 더 이해 편하실 것 같아요.
로직이 많이 복잡하고 읽다 보면 "왜 굳이 이렇게까지 햇지?"라는 생각이 드실 수도 있는데,, 저도 코드 짜면서 이렇게까지 해야되나 생각했씁니다
더 좋은 방법이 보인다면/이해 안 되는 부분이 있다면 마구마구 달아주세요!!!!!!!
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.