Skip to content

Feature/#30 feat timetable name limit #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lluke0 merged 5 commits into from
Aug 30, 2025
Merged

Feature/#30 feat timetable name limit #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b61f4b8
Fix: appBar ๋’ค๋กœ๊ฐ€๊ธฐ ๋ฒ„ํŠผ ํด๋ฆญ ์˜์—ญ ํ™•๋Œ€
BEEEAM-J Aug 19, 2025
ea8a4db
refactor: ์‹œ๊ฐ„ํ‘œ ๋ฆฌ์ŠคํŠธ ๋ฌธ๊ตฌ ์ˆ˜์ •
BEEEAM-J Aug 19, 2025
dc117b5
feat: ์‹œ๊ฐ„ํ‘œ ์ด๋ฆ„ ์ž…๋ ฅ ์œ ํšจ์„ฑ ๊ฒ€์‚ฌ ์ถ”๊ฐ€
BEEEAM-J Aug 19, 2025
afc993a
feat: ์‹œ๊ฐ„ํ‘œ ์ด๋ฆ„ ์ž…๋ ฅ ์œ ํšจ์„ฑ ๊ฒ€์‚ฌ ์ถ”๊ฐ€
BEEEAM-J Aug 25, 2025
d649543
refactor: ์‹œ๊ฐ„ํ‘œ ์ด๋ฆ„ ๊ธ€์ž ์ˆ˜ ์ œํ•œ ๋กœ์ง ๋ณ€๊ฒฝ
BEEEAM-J Aug 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions ...otlin/com/chukchukhaksa/mobile/common/designsystem/component/appbar/CchAppBarWithTitle.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,14 @@ fun CchAppBarWithTitle(
Row(
modifier = Modifier
.wrapContentWidth()
.height(24.dp)
.clickable { onClickBackButton() },
.height(24.dp),
verticalAlignment = Alignment.CenterVertically,
Comment on lines 44 to 47

@coderabbitai coderabbitai Bot Aug 25, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐Ÿ› ๏ธ Refactor suggestion

โš ๏ธ Potential issue

๋’ค๋กœ๊ฐ€๊ธฐ ์˜์—ญ ๋†’์ด 24dp โ†’ ์ตœ์†Œ 48dp๋กœ ์ƒํ–ฅ ํ•„์š”

Row ๋†’์ด๊ฐ€ 24dp๋กœ ๊ณ ์ •๋˜์–ด ์žˆ์–ด ํƒญ ํƒ€๊ฒŸ์ด ๊ถŒ์žฅ ์ตœ์†Œ(48dp)์— ๋ฏธ๋‹ฌํ•ฉ๋‹ˆ๋‹ค. ํด๋ฆญ ์ฒ˜๋ฆฌ๋ฅผ Image๋กœ ์ด๋™ํ•œ ๋งŒํผ, ์ปจํ…Œ์ด๋„ˆ(Row)๋„ ์ตœ์†Œ 48dp ์ด์ƒ์„ ๋ณด์žฅํ•ด์•ผ ์ ‘๊ทผ์„ฑ/์‚ฌ์šฉ์„ฑ ์ €ํ•˜๊ฐ€ ์—†์Šต๋‹ˆ๋‹ค.

์ ์šฉ ์ œ์•ˆ(diff):

-        .height(24.dp),
+        .heightIn(min = 48.dp),

์ถ”๊ฐ€: heightIn ์‚ฌ์šฉ์„ ์œ„ํ•ด import androidx.compose.foundation.layout.heightIn๊ฐ€ ํ•„์š”ํ•ฉ๋‹ˆ๋‹ค.

๐Ÿค– Prompt for AI Agents 
In
composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/common/designsystem/component/appbar/CchAppBarWithTitle.kt
around lines 44โ€“47, the Row for the back area is fixed to height(24.dp) which
yields a tap target below the recommended minimum; replace the fixed height with
heightIn(min = 48.dp) to guarantee at least 48dp touch target and update the
modifier accordingly, and add the import statement import
androidx.compose.foundation.layout.heightIn.

horizontalArrangement = Arrangement.spacedBy(2.dp)
) {
Image(
modifier = Modifier
.clip(CircleShape)
.cchClickable(onClick = onClickBackButton),
Comment on lines +51 to +53

@coderabbitai coderabbitai Bot Aug 25, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐Ÿ› ๏ธ Refactor suggestion

โš ๏ธ Potential issue

ํด๋ฆญ ์˜์—ญ์ด ์•„์ด์ฝ˜ ํฌ๊ธฐ์— ํ•œ์ •๋จ โ€” 48dp ํ„ฐ์น˜ ์˜์—ญ์„ ๋ช…์‹œ์ ์œผ๋กœ ๋ถ€์—ฌํ•˜์„ธ์š”

ํด๋ฆญ์ด Image์—๋งŒ ๊ฑธ๋ฆฌ๋ฉด์„œ ์‹ค์ œ ํ„ฐ์น˜ ์˜์—ญ์ด ์•„์ด์ฝ˜(์ถ”์ • 24dp) ํฌ๊ธฐ๋กœ ์ถ•์†Œ๋˜์—ˆ์Šต๋‹ˆ๋‹ค. ์ตœ์†Œ 48dp์˜ ํ„ฐ์น˜ ์˜์—ญ์„ ๋ณด์žฅํ•˜๋„๋ก size(48.dp)์™€ ๋‚ด๋ถ€ padding(12.dp)์„ ์ ์šฉํ•ด ์ฃผ์„ธ์š”. ์ด๋ ‡๊ฒŒ ํ•˜๋ฉด ์‹œ๊ฐ์  ์•„์ด์ฝ˜์€ 24dp๋กœ ์œ ์ง€๋˜๋ฉด์„œ ํ„ฐ์น˜ ์˜์—ญ์€ 48dp๊ฐ€ ๋ฉ๋‹ˆ๋‹ค.

์ ์šฉ ์ œ์•ˆ(diff):

-        modifier = Modifier
-          .clip(CircleShape)
-          .cchClickable(onClick = onClickBackButton),
+        modifier = Modifier
+          .size(48.dp)
+          .clip(CircleShape)
+          .cchClickable(onClick = onClickBackButton)
+          .padding(12.dp),
๐Ÿ“ 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
modifier = Modifier
.clip(CircleShape)
.cchClickable(onClick = onClickBackButton),
modifier = Modifier
.size(48.dp)
.clip(CircleShape)
.cchClickable(onClick = onClickBackButton)
.padding(12.dp),
๐Ÿค– Prompt for AI Agents 
In
composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/common/designsystem/component/appbar/CchAppBarWithTitle.kt
around lines 51-53, the clickable modifier is only applied to the Image
(reducing touch target to ~24dp); update the modifier chain to guarantee a 48.dp
touch target by adding size(48.dp) and padding(12.dp) so the visible icon stays
24.dp while the touch area is 48.dp, and ensure cchClickable is applied to that
48.dp area (e.g., apply size(48.dp) then cchClickable(onClick =
onClickBackButton) then padding(12.dp)/clip as needed).

painter = painterResource(resource = Res.drawable.ic_appbar_arrow_left_chukchuk),
contentDescription = "",

@coderabbitai coderabbitai Bot Aug 25, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐Ÿ› ๏ธ Refactor suggestion

โš ๏ธ Potential issue

์ ‘๊ทผ์„ฑ: ๋น„์–ด์žˆ๋Š” contentDescription์„ ์˜๋ฏธ ์žˆ๋Š” ๋ฌธ์ž์—ด๋กœ ๊ต์ฒดํ•˜์„ธ์š”

์กฐ์ž‘ ๊ฐ€๋Šฅํ•œ ๋’ค๋กœ๊ฐ€๊ธฐ ๋ฒ„ํŠผ์ธ๋ฐ contentDescription = ""๋กœ ๋˜์–ด ์žˆ์Šต๋‹ˆ๋‹ค. ์Šคํฌ๋ฆฐ๋ฆฌ๋” ์‚ฌ์šฉ์ž๋ฅผ ์œ„ํ•ด ์˜๋ฏธ ์žˆ๋Š” ๋ฌธ์ž์—ด์„ ์ œ๊ณตํ•ด์•ผ ํ•ฉ๋‹ˆ๋‹ค.

์ ์šฉ ์ œ์•ˆ(diff):

-        contentDescription = "",
+        contentDescription = stringResource(Res.string.appbar_navigate_up_content_description),

์ถ”๊ฐ€:

  • import org.jetbrains.compose.resources.stringResource
  • ๋ฌธ์ž์—ด ๋ฆฌ์†Œ์Šค ์˜ˆ์‹œ: appbar_navigate_up_content_description = "๋’ค๋กœ๊ฐ€๊ธฐ" (ํ”Œ๋žซํผ๋ณ„ ํ˜„์ง€ํ™” ๊ฐ€๋Šฅ)
๐Ÿ“ 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
contentDescription = "",
contentDescription = stringResource(Res.string.appbar_navigate_up_content_description),
๐Ÿค– Prompt for AI Agents 
In
composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/common/designsystem/component/appbar/CchAppBarWithTitle.kt
around line 55, the IconButton's contentDescription is an empty string which
breaks accessibility; replace it with a localized stringResource (e.g.
stringResource(R.string.appbar_navigate_up_content_description)) and add the
import org.jetbrains.compose.resources.stringResource, and add a string resource
key (appbar_navigate_up_content_description = "๋’ค๋กœ๊ฐ€๊ธฐ") to the shared resources so
screen readers get a meaningful label.

)
Expand Down
4 changes: 3 additions & 1 deletion ...n/com/chukchukhaksa/mobile/common/designsystem/component/textfield/CchRegularTextField.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import com.chukchukhaksa.mobile.common.designsystem.theme.CchTheme
import com.chukchukhaksa.mobile.common.designsystem.theme.Gray200
import com.chukchukhaksa.mobile.common.designsystem.theme.Gray400
import com.chukchukhaksa.mobile.common.designsystem.theme.Purple600
import com.chukchukhaksa.mobile.common.designsystem.theme.Red300
import com.chukchukhaksa.mobile.common.designsystem.theme.White100

@Composable
Expand All @@ -32,13 +33,14 @@ fun CchRegularTextField(
value: String = "",
placeholder: String = "",
isActive: Boolean = false,
isError: Boolean = false,
onValueChanged: (String) -> Unit = {},
onClickClearButton: () -> Unit = {}
) {
val (borderColor, textColor, textStyle) = if(!isActive) {
Triple(Gray200, Gray400, CchTheme.typography.bodyLg)
} else {
Triple(Purple600, Black100, CchTheme.typography.bodyLgStrong)
Triple(if (isError) Red300 else Purple600, Black100, CchTheme.typography.bodyLgStrong)
}
Comment on lines 40 to 44

@coderabbitai coderabbitai Bot Aug 25, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐Ÿ› ๏ธ Refactor suggestion

์—๋Ÿฌ ์ƒํƒœ๊ฐ€ ๋น„ํ™œ์„ฑ(isActive=false)์ผ ๋•Œ ๋นจ๊ฐ„ ๋ณด๋”๊ฐ€ ํ‘œ์‹œ๋˜์ง€ ์•Š์Œ โ€” ์—๋Ÿฌ ์šฐ์„  ๋กœ์ง์œผ๋กœ ์ •์ •

ํ˜„์žฌ๋Š” isActive๊ฐ€ false๋ฉด ์—๋Ÿฌ์—ฌ๋„ ํšŒ์ƒ‰ ๋ณด๋”๊ฐ€ ๋ณด์—ฌ ์‚ฌ์šฉ์ž ํ”ผ๋“œ๋ฐฑ์ด ์•ฝํ•ฉ๋‹ˆ๋‹ค. ์—๋Ÿฌ์ผ ๋•Œ๋Š” ํ™œ์„ฑ/๋น„ํ™œ์„ฑ ๋ฌด๊ด€ํ•˜๊ฒŒ Red300์ด ๋ณด์ด๋„๋ก ์กฐ๊ฑด์‹์„ ์ •๋ฆฌํ•˜์„ธ์š”.

์ ์šฉ ์˜ˆ์‹œ(diff):

-  val (borderColor, textColor, textStyle) = if(!isActive) {
-    Triple(Gray200, Gray400, CchTheme.typography.bodyLg)
-  } else {
-    Triple(if (isError) Red300 else Purple600, Black100, CchTheme.typography.bodyLgStrong)
-  }
+  val borderColor = when {
+    isError -> Red300
+    isActive -> Purple600
+    else -> Gray200
+  }
+  val textColor = if (isActive) Black100 else Gray400
+  val textStyle = if (isActive) CchTheme.typography.bodyLgStrong else CchTheme.typography.bodyLg
๐Ÿ“ 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
val (borderColor, textColor, textStyle) = if(!isActive) {
Triple(Gray200, Gray400, CchTheme.typography.bodyLg)
} else {
Triple(Purple600, Black100, CchTheme.typography.bodyLgStrong)
Triple(if (isError) Red300 else Purple600, Black100, CchTheme.typography.bodyLgStrong)
}
// Before: destructuring all three values based on isActive/error
- val (borderColor, textColor, textStyle) = if(!isActive) {
- Triple(Gray200, Gray400, CchTheme.typography.bodyLg)
- } else {
- Triple(if (isError) Red300 else Purple600, Black100, CchTheme.typography.bodyLgStrong)
- }
// After: error-first borderColor, then active/inactive; separate textColor and textStyle
val borderColor = when {
isError -> Red300
isActive -> Purple600
else -> Gray200
}
val textColor = if (isActive) Black100 else Gray400
val textStyle = if (isActive) CchTheme.typography.bodyLgStrong else CchTheme.typography.bodyLg
๐Ÿค– Prompt for AI Agents 
In
composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/common/designsystem/component/textfield/CchRegularTextField.kt
around lines 40 to 44, the conditional currently hides the error border when
isActive is false; change the logic to make error take precedence: compute
borderColor as if (isError) Red300 else if (!isActive) Gray200 else Purple600,
and keep textColor/textStyle determined by isActive (Gray400/bodyLg when
inactive, Black100/bodyLgStrong when active). Update the Triple construction
accordingly so error always shows Red300 regardless of isActive.

BasicTextField(
value = value,
Expand Down
11 changes: 11 additions & 0 deletions ...App/src/commonMain/kotlin/com/chukchukhaksa/mobile/common/extension/TimetableNameLimit.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.chukchukhaksa.mobile.common.extension

const val TIMETABLE_NAME_LIMIT = 20

fun checkOverTimetableNameLimit(name: String): Boolean {
return name.trim().length > TIMETABLE_NAME_LIMIT
}

fun checkTimetableNameRule(name: String): Boolean {
return name.isNotBlank() && name.trim().length <= TIMETABLE_NAME_LIMIT
}
Comment on lines +3 to +11

@coderabbitai coderabbitai Bot Aug 26, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐Ÿงน Nitpick (assertive)

๋ฌธ์ž ๊ธธ์ด ๊ธฐ์ค€(UTF-16 length vs. ์‚ฌ์šฉ์ž ์ฒด๊ฐ ๊ธ€์ž์ˆ˜) ํ™•์ธ ํ•„์š”

ํ˜„์žฌ length ๊ธฐ์ค€์€ UTFโ€‘16 ์ฝ”๋“œ ์œ ๋‹› ์ˆ˜์ž…๋‹ˆ๋‹ค. ์ด๋ชจ์ง€/์กฐํ•ฉํ˜• ๋ฌธ์ž(์˜ˆ: ํ”ผ๋ถ€ํ†ค ์ด๋ชจ์ง€, ํ•ฉ์„ฑ ํ”Œ๋ž˜๊ทธ)์—์„œ๋Š” ์‚ฌ์šฉ์ž ์ฒด๊ฐ ๊ธ€์ž์ˆ˜์™€ ์–ด๊ธ‹๋‚  ์ˆ˜ ์žˆ์Šต๋‹ˆ๋‹ค. ์š”๊ตฌ์‚ฌํ•ญ์ด โ€œ์‹œ๊ฐ์ ์œผ๋กœ 20์žโ€๋ฅผ ์˜๋ฏธํ•œ๋‹ค๋ฉด, KMP ํ™˜๊ฒฝ์—์„œ์˜ ๊ทธ๋ผํŽ˜๋ฉ” ํด๋Ÿฌ์Šคํ„ฐ ์นด์šดํŒ… ์ „๋žต(ํ”Œ๋žซํผ๋ณ„ ICU/Regex) ๊ฒ€ํ† ๊ฐ€ ํ•„์š”ํ•ฉ๋‹ˆ๋‹ค. ํ˜„ ๋‹จ๊ณ„์—์„œ๋Š” ์ •์ฑ… ํ™•์ • ์ „๊นŒ์ง€๋Š” ํ˜„ ๊ตฌํ˜„ ์œ ์ง€ + ๋ช…ํ™•ํ•œ ์ŠคํŽ™ ์ฃผ์„ ์ถ”๊ฐ€๋ฅผ ๊ถŒ์žฅํ•ฉ๋‹ˆ๋‹ค.

๐Ÿงน Nitpick (assertive)

๋ถˆ๋ฆฌ์–ธ ๊ทœ์น™ ํ•จ์ˆ˜ ๋„ค์ด๋ฐ๊ณผ ์œ„์น˜ ์ผ๊ด€์„ฑ ๊ฐœ์„  ์ œ์•ˆ

  • ๋„ค์ด๋ฐ: checkOverTimetableNameLimit/checkTimetableNameRule๋Š” ๊ฒฐ๊ณผ๊ฐ€ true/false์ธ ๊ฒ€์ฆ๊ธฐ์ด๋ฏ€๋กœ ์ˆ ์–ดํ˜•(is*)์ด ๊ฐ€๋…์„ฑ์ด ๋” ์ข‹์Šต๋‹ˆ๋‹ค.
    ์˜ˆ: isTimetableNameOverLimit, isTimetableNameValid.
  • ํŒจํ‚ค์ง€/์œ„์น˜: ๋ณธ ํŒŒ์ผ์€ ํ™•์žฅ ํ•จ์ˆ˜๊ฐ€ ์•„๋‹Œ ๊ณตํ†ต ์œ ํ‹ธ์ž…๋‹ˆ๋‹ค. ๊ธฐ์กด ๊ฐ€์ด๋“œ(learned) ์ƒ util ๋””๋ ‰ํ„ฐ๋ฆฌ๋กœ์˜ ์ด๋™์„ ๊ถŒ์žฅํ•ฉ๋‹ˆ๋‹ค: common/util/...๋กœ ์ด๋™ํ•˜๋ฉด ๊ฒ€์ƒ‰์„ฑ๊ณผ ์‘์ง‘๋„๊ฐ€ ์ข‹์•„์ง‘๋‹ˆ๋‹ค.

์•„๋ž˜์™€ ๊ฐ™์ด ๋„ค์ด๋ฐ์„ ์ •๋ฆฌํ•˜๋ฉด ํ˜ธ์ถœ๋ถ€ ๊ฐ€๋…์„ฑ์ด ๊ฐœ์„ ๋ฉ๋‹ˆ๋‹ค.

-const val TIMETABLE_NAME_LIMIT = 20
-
-fun checkOverTimetableNameLimit(name: String): Boolean {
-    return name.trim().length > TIMETABLE_NAME_LIMIT
-}
-
-fun checkTimetableNameRule(name: String): Boolean {
-    return name.isNotBlank() && name.trim().length <= TIMETABLE_NAME_LIMIT
-}
+const val TIMETABLE_NAME_LIMIT = 20
+
+fun isTimetableNameOverLimit(name: String): Boolean =
+    name.trim().length > TIMETABLE_NAME_LIMIT
+
+fun isTimetableNameValid(name: String): Boolean =
+    name.isNotBlank() && name.trim().length <= TIMETABLE_NAME_LIMIT

ํ˜ธ์ถœ๋ถ€ ๋ณ€๊ฒฝ์€ ๋ณธ PR์˜ ๋‘ Screen/๋‘ Contract์—์„œ๋งŒ ํ•„์š”ํ•ด ์˜ํ–ฅ ๋ฒ”์œ„๊ฐ€ ์ž‘์Šต๋‹ˆ๋‹ค. ์›ํ•˜์‹œ๋ฉด ๊ด€๋ จ ํ˜ธ์ถœ๋ถ€ diff๋„ ํ•จ๊ป˜ ์ œ์•ˆ๋“œ๋ฆฌ๊ฒ ์Šต๋‹ˆ๋‹ค.

๐Ÿ“ 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
const val TIMETABLE_NAME_LIMIT = 20
fun checkOverTimetableNameLimit(name: String): Boolean {
return name.trim().length > TIMETABLE_NAME_LIMIT
}
fun checkTimetableNameRule(name: String): Boolean {
return name.isNotBlank() && name.trim().length <= TIMETABLE_NAME_LIMIT
}
const val TIMETABLE_NAME_LIMIT = 20
fun isTimetableNameOverLimit(name: String): Boolean =
name.trim().length > TIMETABLE_NAME_LIMIT
fun isTimetableNameValid(name: String): Boolean =
name.isNotBlank() && name.trim().length <= TIMETABLE_NAME_LIMIT
๐Ÿค– Prompt for AI Agents 
In
composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/common/extension/TimetableNameLimit.kt
around lines 3-11, the boolean validator functions use non-predicate names and
live in an extensions package though they are general utilities; rename
checkOverTimetableNameLimit -> isTimetableNameOverLimit and
checkTimetableNameRule -> isTimetableNameValid, keep the same logic, move the
file into common/util (e.g.,
composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/common/util/TimetableNameLimit.kt)
and update the two Screens/Contracts that call these functions to use the new
names and import path.

3 changes: 2 additions & 1 deletion ...om/chukchukhaksa/mobile/presentation/timetable/timetableeditor/TimetableEditorContract.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.chukchukhaksa.mobile.presentation.timetable.timetableeditor
import com.chukchukhaksa.mobile.presentation.timetable.navigation.argument.TimetableEditorArgument
import com.chukchukhaksa.mobile.presentation.timetable.semesterselect.semesterList
import com.chukchukhaksa.mobile.presentation.timetable.semesterselect.Semester
import com.chukchukhaksa.mobile.common.extension.checkTimetableNameRule

@coderabbitai coderabbitai Bot Aug 26, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐Ÿงน Nitpick (assertive)

์—๋””ํ„ฐ ๋ฒ„ํŠผ ํ™œ์„ฑํ™” ๊ทœ์น™ ๊ณตํ†ตํ™” ์ ์šฉ ๐Ÿ‘

checkTimetableNameRule(name)(= ๊ณต๋ฐฑ ๋ฐฉ์ง€ + ํŠธ๋ฆผ ํ›„ ๊ธธ์ด ์ œํ•œ)์™€ ๋ณ€๊ฒฝ ์œ ๋ฌด ์ฒดํฌ๋ฅผ ํ•จ๊ป˜ ์ ์šฉํ•œ ๊ตฌ์„ฑ์€ ๋ช…ํ™•ํ•ฉ๋‹ˆ๋‹ค. ๊ด„ํ˜ธ๋Š” ์ค‘๋ณต์ด๋ฏ€๋กœ ๊ฐ„๊ฒฐํ™” ์—ฌ์ง€๋Š” ์žˆ์ง€๋งŒ ๊ธฐ๋Šฅ์ƒ ๋ฌธ์ œ๋Š” ์—†์Šต๋‹ˆ๋‹ค.

-    val buttonEnabled = ((checkTimetableNameRule(name)) && (preName != name || preSelectedSemesterPosition != selectedSemesterPosition))
+    val buttonEnabled = checkTimetableNameRule(name) &&
+        (preName != name || preSelectedSemesterPosition != selectedSemesterPosition)

Also applies to: 17-17

๐Ÿค– Prompt for AI Agents 
In
composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/presentation/timetable/timetableeditor/TimetableEditorContract.kt
around lines 6 and 17, there are redundant parentheses around the condition that
uses checkTimetableNameRule(name) together with the change-check; remove the
extra parentheses to simplify the boolean expression (e.g., change from
((checkTimetableNameRule(name)) && changed) or (checkTimetableNameRule(name) &&
changed) with extra surrounding parens to simply checkTimetableNameRule(name) &&
changed) while keeping the same logic and spacing.



data class TimetableEditorState(
Expand All @@ -13,7 +14,7 @@ data class TimetableEditorState(
val selectedSemesterPosition: Int? = null,
) {
val semester = selectedSemesterPosition?.let { semesterList.getOrNull(it) }
val buttonEnabled = (name.isNotEmpty() && preName != name || preSelectedSemesterPosition != selectedSemesterPosition)
val buttonEnabled = ((checkTimetableNameRule(name)) && (preName != name || preSelectedSemesterPosition != selectedSemesterPosition))
}

internal fun TimetableEditorArgument.toState() = TimetableEditorState(
Expand Down
19 changes: 19 additions & 0 deletions .../com/chukchukhaksa/mobile/presentation/timetable/timetableeditor/TimetableEditorScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@ import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.WindowInsets
import androidx.compose.foundation.layout.consumeWindowInsets
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.imePadding
import androidx.compose.foundation.layout.navigationBars
import androidx.compose.foundation.layout.padding
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.dp
import androidx.lifecycle.compose.collectAsStateWithLifecycle
Expand All @@ -23,7 +26,10 @@ import com.chukchukhaksa.mobile.common.designsystem.component.bottomsheet.CchSel
import com.chukchukhaksa.mobile.common.designsystem.component.button.CchBasicButton
import com.chukchukhaksa.mobile.common.designsystem.component.container.CchSelectionButton
import com.chukchukhaksa.mobile.common.designsystem.component.textfield.CchRegularTextField
import com.chukchukhaksa.mobile.common.designsystem.theme.CchTheme
import com.chukchukhaksa.mobile.common.designsystem.theme.Red300
import com.chukchukhaksa.mobile.common.designsystem.theme.White
import com.chukchukhaksa.mobile.common.extension.checkOverTimetableNameLimit
import com.chukchukhaksa.mobile.common.ui.collectWithLifecycle
import com.chukchukhaksa.mobile.presentation.timetable.semesterselect.semesterList
import kotlinx.collections.immutable.toPersistentList
Expand Down Expand Up @@ -100,10 +106,23 @@ fun TimetableEditorScreen(
value = uiState.name,
placeholder = stringResource(Res.string.create_timetable_screen_placeholder),
isActive = uiState.name.isNotEmpty() && uiState.name != uiState.preName,
isError = checkOverTimetableNameLimit(uiState.name),
onValueChanged = onValueChangeTimetableName,
onClickClearButton = onClickTextFieldClearButton,
)
Comment on lines +109 to 112

@coderabbitai coderabbitai Bot Aug 26, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐Ÿ› ๏ธ Refactor suggestion

โš ๏ธ Potential issue

์—๋Ÿฌ ํ‘œ์‹œ ์กฐ๊ฑด ๋ถˆ์ผ์น˜์™€ ํ•˜๋“œ์ฝ”๋”ฉ(20) ์ค‘๋ณต โ€” ์ž…๋ ฅ ํ™”๋ฉด๊ณผ ๋™์ผ ์ด์Šˆ

์ž…๋ ฅ ํ™”๋ฉด๊ณผ ๋™์ผํ•˜๊ฒŒ isError(trim ๊ธฐ์ค€)์™€ ์—๋Ÿฌ ํ…์ŠคํŠธ ํ‘œ์‹œ(์›๋ฌธ length > 20)์˜ ๊ธฐ์ค€์ด ๋‹ฌ๋ผ์ง‘๋‹ˆ๋‹ค. ๊ณตํ†ต ํ•จ์ˆ˜/์ƒ์ˆ˜๋ฅผ ์‚ฌ์šฉํ•ด ์ •ํ•ฉ์„ฑ์„ ๋งž์ถฐ์ฃผ์„ธ์š”.

-                    isError = checkOverTimetableNameLimit(uiState.name),
+                    isError = checkOverTimetableNameLimit(uiState.name),
...
-                if (uiState.name.length > 20) {
+                if (checkOverTimetableNameLimit(uiState.name)) {
                   Text(
                     modifier = Modifier
                       .padding(top = 12.dp, start = 4.dp, end = 4.dp)
                       .fillMaxWidth(),
-                    text = "์‹œ๊ฐ„ํ‘œ ์ด๋ฆ„์€ ์ตœ๋Œ€ 20์ž๊นŒ์ง€ ์„ค์ • ๊ฐ€๋Šฅํ•ฉ๋‹ˆ๋‹ค.",
+                    text = "์‹œ๊ฐ„ํ‘œ ์ด๋ฆ„์€ ์ตœ๋Œ€ ${TIMETABLE_NAME_LIMIT}์ž๊นŒ์ง€ ์„ค์ • ๊ฐ€๋Šฅํ•ฉ๋‹ˆ๋‹ค.",
                     textAlign = TextAlign.Start,
                     style = CchTheme.typography.bodyMd,
                     color = Red300,
                   )
                 }

ํ•„์š”ํ•œ import:

 import com.chukchukhaksa.mobile.common.extension.checkOverTimetableNameLimit
+import com.chukchukhaksa.mobile.common.extension.TIMETABLE_NAME_LIMIT

Also applies to: 114-124

๐Ÿค– Prompt for AI Agents 
In
composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/presentation/timetable/timetableeditor/TimetableEditorScreen.kt
around lines 109-112 (and also apply to 114-124), the isError check uses a
different criterion than the error-text visibility and the hardcoded limit 20 is
duplicated; refactor to a shared constant (e.g., TIMETABLE_NAME_MAX_LENGTH) and
a shared trim-aware validator function (e.g., isOverTimetableNameLimit(name:
String): Boolean) imported from the common validation/constants file, then
replace the inline isError and the length > 20 checks to use that single
function/constant so both the error flag and displayed message use the same
trim-based threshold; update imports to include the constant/function and adjust
any error message formatting to reference the constant instead of the hardcoded
20.


if (uiState.name.length > 20) {
Text(
modifier = Modifier
.padding(top = 12.dp, start = 4.dp, end = 4.dp)
.fillMaxWidth(),
text = "์‹œ๊ฐ„ํ‘œ ์ด๋ฆ„์€ ์ตœ๋Œ€ 20์ž๊นŒ์ง€ ์„ค์ • ๊ฐ€๋Šฅํ•ฉ๋‹ˆ๋‹ค.",
textAlign = TextAlign.Start,
style = CchTheme.typography.bodyMd,
color = Red300,
)
}
Comment on lines +114 to +124

@coderabbitai coderabbitai Bot Aug 26, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐Ÿงน Nitpick (assertive)

๋ฌธ๊ตฌ ํ•˜๋“œ์ฝ”๋”ฉ โ†’ ๋ฆฌ์†Œ์Šคํ™” ๊ณ ๋ ค (๋™์ผ ์ œ์•ˆ)

์ž…๋ ฅ ํ™”๋ฉด๊ณผ ๋™์ผํ•˜๊ฒŒ ๋ฌธ์ž์—ด ๋ฆฌ์†Œ์Šค ์‚ฌ์šฉ์„ ๊ถŒ์žฅํ•ฉ๋‹ˆ๋‹ค. ์ž๋ฆฌํ‘œ์‹œ์ž์— TIMETABLE_NAME_LIMIT ์ฃผ์ž… ๋ฐฉ์‹์œผ๋กœ ๊ตญ์ œํ™” ๋Œ€๋น„๊ฐ€ ์šฉ์ดํ•ฉ๋‹ˆ๋‹ค.

๐Ÿค– Prompt for AI Agents 
In
composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/presentation/timetable/timetableeditor/TimetableEditorScreen.kt
around lines 114โ€“124, the displayed Korean message is hardcoded; extract it into
a string resource (e.g., TIMETABLE_NAME_LIMIT) and replace the literal with a
localized lookup (use stringResource or your multiplatform resource helper and
inject the limit placeholder value). Add the new resource entry in your strings
resources (with a placeholder for the max length) and update the UI code to call
the resource with the max length (20) instead of the hardcoded text.


Spacer(modifier = Modifier.weight(1f))

CchBasicButton(
Expand Down
4 changes: 2 additions & 2 deletions ...tlin/com/chukchukhaksa/mobile/presentation/timetable/timetablelist/TimetableListScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ fun TimetableListScreen(
if (uiState.timetableList.isEmpty()) {
Text(
modifier = Modifier
.padding(top = 150.dp),
.padding(top = 324.dp),
textAlign = TextAlign.Center,
text = stringResource(Res.string.timetable_list_screen_empty_timetable),
text = "๋“ฑ๋ก๋œ ์‹œ๊ฐ„ํ‘œ๊ฐ€ ์—†์–ด์š”!",
style = CchTheme.typography.bodyMd,
color = Gray600,
)
Comment on lines +109 to 112

@coderabbitai coderabbitai Bot Aug 25, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐Ÿ’ก Verification agent

๐Ÿงฉ Analysis chain

๋ฌธ์ž์—ด ํ•˜๋“œ์ฝ”๋”ฉ์œผ๋กœ i18n ๊นจ์ง โ€” stringResource ์‚ฌ์šฉ์œผ๋กœ ๋ณต๊ตฌ ํ•„์š” (R)

  • Line 109์—์„œ ํ•˜๋“œ์ฝ”๋”ฉ๋œ ๋ฌธ์ž์—ด์€ ๋‹ค๊ตญ์–ด/๋กœ์ปฌ๋ผ์ด์ œ์ด์…˜์„ ๊นจ๋œจ๋ฆฝ๋‹ˆ๋‹ค. ๊ธฐ์กด์ฒ˜๋Ÿผ stringResource๋ฅผ ์‚ฌ์šฉํ•ด ์ฃผ์„ธ์š”. ๊ธฐํš ์˜๋„๊ฐ€ ๋ฌธ๊ตฌ ๋ณ€๊ฒฝ์ด๋ผ๋ฉด ๋ฆฌ์†Œ์Šค ๊ฐ’ ์ž์ฒด๋ฅผ ๊ฐฑ์‹ ํ•˜๊ณ  ๋ฒˆ์—ญ ํ‚ค๋ฅผ ์œ ์ง€/์ถ”๊ฐ€ํ•˜์„ธ์š”.

์›ํ•˜์‹œ๋ฉด ๋ฆฌ์†Œ์Šค ํ‚ค/๋ฒˆ์—ญ ์ถ”๊ฐ€ PR ์Šค์บํด๋”ฉ๊นŒ์ง€ ์ค€๋น„ํ•ด ๋“œ๋ฆฝ๋‹ˆ๋‹ค.

-                    text = "๋“ฑ๋ก๋œ ์‹œ๊ฐ„ํ‘œ๊ฐ€ ์—†์–ด์š”!",
+                    text = stringResource(Res.string.timetable_list_screen_empty_timetable),

์ถ”๊ฐ€ ๊ฒ€์ฆ์„ ์œ„ํ•ด ์•„๋ž˜ ์Šคํฌ๋ฆฝํŠธ๋ฅผ ์‹คํ–‰ํ•ด ์ฃผ์„ธ์š”.
ํ•ด๋‹น ๋ฆฌ์†Œ์Šค ํ‚ค์˜ ์กด์žฌ ์—ฌ๋ถ€์™€ stringResource ์‚ฌ์šฉ ํŒจํ„ด์„ ํ™•์ธํ•ฉ๋‹ˆ๋‹ค.

๐Ÿ Script executed:

#!/bin/bash
# 1. ๋ฆฌ์†Œ์Šค ํŒŒ์ผ ๊ฒ€์ƒ‰
find . -type f -name strings.xml

# 2. ํ‚ค ์กด์žฌ ์—ฌ๋ถ€ ํ™•์ธ
rg -n 'timetable_list_screen_empty_timetable' .

# 3. TimetableListScreen.kt์˜ import ๋ฐ stringResource ์‚ฌ์šฉ ์—ฌ๋ถ€ ํ™•์ธ
sed -n '1,200p' composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/presentation/timetable/timetablelist/TimetableListScreen.kt

# 4. ํ”„๋กœ์ ํŠธ ์ „๋ฐ˜์˜ stringResource ์‚ฌ์šฉ ํŒจํ„ด ๊ฒ€์ƒ‰
rg -n 'stringResource' -C2 composeApp

Length of output: 45853

ํ•˜๋“œ์ฝ”๋”ฉ๋œ ๋ฌธ์ž์—ด ์ œ๊ฑฐ ๋ฐ stringResource ์ ์šฉ ํ•„์š”
109ํ–‰์˜ Text ์ปดํฌ์ €๋ธ”์— ํ•˜๋“œ์ฝ”๋”ฉ๋œ "๋“ฑ๋ก๋œ ์‹œ๊ฐ„ํ‘œ๊ฐ€ ์—†์–ด์š”!" ๋ฌธ์ž์—ด์ด ๋‚จ์•„ ์žˆ์–ด ๋‹ค๊ตญ์–ด/๋กœ์ปฌ๋ผ์ด์ œ์ด์…˜(i18n)์ด ๊นจ์ง‘๋‹ˆ๋‹ค. ์ด๋ฏธ timetable_list_screen_empty_timetable ๋ฆฌ์†Œ์Šค(commonMain/composeResources/values/strings.xml 150ํ–‰)์— ๊ธฐ๋ณธ๊ฐ’ "์‹œ๊ฐ„ํ‘œ๊ฐ€ ์—†์–ด์š”"๋กœ ์ •์˜๋˜์–ด ์žˆ์œผ๋ฏ€๋กœ, ํ™”๋ฉด์— ํ‘œ์‹œํ•  ๋ฌธ๊ตฌ๊ฐ€ ๋ณ€๊ฒฝ๋œ ๊ฒŒ ๋ชฉ์ ์ด๋ผ๋ฉด ๋ฆฌ์†Œ์Šค ๊ฐ’์„ "๋“ฑ๋ก๋œ ์‹œ๊ฐ„ํ‘œ๊ฐ€ ์—†์–ด์š”!"๋กœ ๊ฐฑ์‹ ํ•˜๊ณ  stringResource๋กœ ์ฐธ์กฐํ•ด์•ผ ํ•ฉ๋‹ˆ๋‹ค.

โ€ข ๋Œ€์ƒ ์œ„์น˜

  • ํŒŒ์ผ: composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/presentation/timetable/timetablelist/TimetableListScreen.kt
  • ๋ผ์ธ: 109โ€“112 (Text ์ปดํฌ์ €๋ธ”์˜ text ์†์„ฑ)

โ€ข ํ™•์ธ ์‚ฌํ•ญ

  1. commonMain/composeResources/values/strings.xml์— timetable_list_screen_empty_timetable ์ •์˜ ์—ฌ๋ถ€ ๋ฐ ๊ฐ’(ํ˜„์žฌ "์‹œ๊ฐ„ํ‘œ๊ฐ€ ์—†์–ด์š”")
  2. ๋ฆฌ์†Œ์Šค ๊ฐ’์„ "๋“ฑ๋ก๋œ ์‹œ๊ฐ„ํ‘œ๊ฐ€ ์—†์–ด์š”!"๋กœ ์—…๋ฐ์ดํŠธํ• ์ง€ ๊ฒฐ์ •
  3. ์ˆ˜์ • ํ›„ Text ์ปดํฌ์ €๋ธ”์— stringResource ํ˜ธ์ถœ ์ ์šฉ
@@ composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/presentation/timetable/timetablelist/TimetableListScreen.kt:109
-                Text(
-                    modifier = Modifier
-                        .padding(top = 324.dp),
-                    textAlign = TextAlign.Center,
-                    text = "๋“ฑ๋ก๋œ ์‹œ๊ฐ„ํ‘œ๊ฐ€ ์—†์–ด์š”!",
-                    style = CchTheme.typography.bodyMd,
-                    color = Gray600,
-                )
+                Text(
+                    modifier = Modifier
+                        .padding(top = 324.dp),
+                    textAlign = TextAlign.Center,
+                    text = stringResource(Res.string.timetable_list_screen_empty_timetable),
+                    style = CchTheme.typography.bodyMd,
+                    color = Gray600,
+                )

โ€” ๋ฆฌ์†Œ์Šค ๊ฐฑ์‹  ๋ฐ ์ฐธ์กฐ ์ ์šฉ์— ์–ด๋ ค์›€์ด ์žˆ์œผ๋ฉด ์•Œ๋ ค์ฃผ์„ธ์š”. PR ์Šค์บํด๋”ฉ ์ง€์› ๊ฐ€๋Šฅํ•ฉ๋‹ˆ๋‹ค.

๐Ÿ“ 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
text = "๋“ฑ๋ก๋œ ์‹œ๊ฐ„ํ‘œ๊ฐ€ ์—†์–ด์š”!",
style = CchTheme.typography.bodyMd,
color = Gray600,
)
Text(
modifier = Modifier
.padding(top = 324.dp),
textAlign = TextAlign.Center,
text = stringResource(Res.string.timetable_list_screen_empty_timetable),
style = CchTheme.typography.bodyMd,
color = Gray600,
)
๐Ÿค– Prompt for AI Agents 
composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/presentation/timetable/timetablelist/TimetableListScreen.kt
lines 109โ€“112: Text composable currently uses a hardcoded string "๋“ฑ๋ก๋œ ์‹œ๊ฐ„ํ‘œ๊ฐ€ ์—†์–ด์š”!"
which breaks localization; replace the hardcoded text with a stringResource
lookup (R or resource key) referencing timetable_list_screen_empty_timetable,
and if you intend the displayed wording to be "๋“ฑ๋ก๋œ ์‹œ๊ฐ„ํ‘œ๊ฐ€ ์—†์–ด์š”!" update
commonMain/composeResources/values/strings.xml entry
timetable_list_screen_empty_timetable to that value; finally rebuild to ensure
the resource key is resolved and the Text uses stringResource(...) instead of a
raw string.

Expand Down
4 changes: 2 additions & 2 deletions ...kchukhaksa/mobile/presentation/timetable/timetablenameinput/TimetableNameInputContract.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
package com.chukchukhaksa.mobile.presentation.timetable.timetablenameinput

import com.chukchukhaksa.mobile.common.extension.checkTimetableNameRule
import com.chukchukhaksa.mobile.presentation.timetable.navigation.argument.TimetableEditorArgument
import com.chukchukhaksa.mobile.presentation.timetable.semesterselect.semesterList
import com.chukchukhaksa.mobile.presentation.timetable.semesterselect.Semester


data class TimetableNameInputState(
val name: String = "",
val semester: Semester = Semester("", "")
) {
val buttonEnabled = name.isNotEmpty()
val buttonEnabled = checkTimetableNameRule(name)
}

internal fun TimetableEditorArgument.toState() = TimetableNameInputState(
Expand Down
16 changes: 16 additions & 0 deletions ...hukchukhaksa/mobile/presentation/timetable/timetablenameinput/TimetableNameInputScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.WindowInsets
import androidx.compose.foundation.layout.consumeWindowInsets
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.imePadding
import androidx.compose.foundation.layout.navigationBars
import androidx.compose.foundation.layout.padding
Expand All @@ -28,7 +29,9 @@ import com.chukchukhaksa.mobile.common.designsystem.component.button.CchBasicBut
import com.chukchukhaksa.mobile.common.designsystem.component.textfield.CchRegularTextField
import com.chukchukhaksa.mobile.common.designsystem.theme.Black100
import com.chukchukhaksa.mobile.common.designsystem.theme.CchTheme
import com.chukchukhaksa.mobile.common.designsystem.theme.Red300
import com.chukchukhaksa.mobile.common.designsystem.theme.White
import com.chukchukhaksa.mobile.common.extension.checkOverTimetableNameLimit
import com.chukchukhaksa.mobile.common.ui.collectWithLifecycle
import org.jetbrains.compose.resources.getString
import org.jetbrains.compose.resources.stringResource
Expand Down Expand Up @@ -102,10 +105,23 @@ fun TimetableNameInputScreen(
value = uiState.name,
placeholder = stringResource(Res.string.create_timetable_screen_placeholder),
isActive = uiState.name.isNotEmpty(),
isError = checkOverTimetableNameLimit(uiState.name),
onValueChanged = onValueChangeTimetableName,
onClickClearButton = onClickTextFieldClearButton,
)
Comment on lines +108 to 111

@coderabbitai coderabbitai Bot Aug 26, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐Ÿ› ๏ธ Refactor suggestion

โš ๏ธ Potential issue

์—๋Ÿฌ ํ‘œ์‹œ ์กฐ๊ฑด ๋ถˆ์ผ์น˜์™€ ํ•˜๋“œ์ฝ”๋”ฉ(20) ์ค‘๋ณต โ€” ๋‹จ์ผ ์†Œ์Šค ์‚ฌ์šฉ์œผ๋กœ ์ •ํ•ฉ์„ฑ ๋ณด์žฅ ํ•„์š”

  • ํ…์ŠคํŠธํ•„๋“œ isError๋Š” checkOverTimetableNameLimit(uiState.name)(trim ๊ธฐ์ค€)์ธ๋ฐ, ์•„๋ž˜ ์—๋Ÿฌ ๋ฌธ๊ตฌ ํ‘œ์‹œ๋Š” uiState.name.length > 20(trim ๋ฏธ์ ์šฉ)๋กœ ๋‹ฌ๋ผ UX ํ˜ผ๋ž€์ด ์ƒ๊น๋‹ˆ๋‹ค.
  • ๋ฉ”์‹œ์ง€์˜ ์ˆซ์ž 20์ด ํ•˜๋“œ์ฝ”๋”ฉ๋˜์–ด ์œ ์ง€๋ณด์ˆ˜์„ฑ์ด ๋–จ์–ด์ง‘๋‹ˆ๋‹ค.

๋™์ผํ•œ ๊ฒ€์ฆ ํ•จ์ˆ˜์™€ ๊ณตํ†ต ์ƒ์ˆ˜๋ฅผ ์‚ฌ์šฉํ•˜๋„๋ก ์•„๋ž˜์™€ ๊ฐ™์ด ์ •๋ฆฌํ•ด์ฃผ์„ธ์š”.

-                    isError = checkOverTimetableNameLimit(uiState.name),
+                    isError = checkOverTimetableNameLimit(uiState.name),
...
-                if (uiState.name.length > 20) {
+                if (checkOverTimetableNameLimit(uiState.name)) {
                   Text(
                     modifier = Modifier
                       .padding(top = 12.dp, start = 4.dp, end = 4.dp)
                       .fillMaxWidth(),
-                    text = "์‹œ๊ฐ„ํ‘œ ์ด๋ฆ„์€ ์ตœ๋Œ€ 20์ž๊นŒ์ง€ ์„ค์ • ๊ฐ€๋Šฅํ•ฉ๋‹ˆ๋‹ค.",
+                    text = "์‹œ๊ฐ„ํ‘œ ์ด๋ฆ„์€ ์ตœ๋Œ€ ${TIMETABLE_NAME_LIMIT}์ž๊นŒ์ง€ ์„ค์ • ๊ฐ€๋Šฅํ•ฉ๋‹ˆ๋‹ค.",
                     textAlign = TextAlign.Start,
                     style = CchTheme.typography.bodyMd,
                     color = Red300,
                   )
                 }

ํ•„์š”ํ•œ import ์ถ”๊ฐ€:

 import com.chukchukhaksa.mobile.common.extension.checkOverTimetableNameLimit
+import com.chukchukhaksa.mobile.common.extension.TIMETABLE_NAME_LIMIT

Also applies to: 113-123


if (uiState.name.length > 20) {
Text(
modifier = Modifier
.padding(top = 12.dp, start = 4.dp, end = 4.dp)
.fillMaxWidth(),
text = "์‹œ๊ฐ„ํ‘œ ์ด๋ฆ„์€ ์ตœ๋Œ€ 20์ž๊นŒ์ง€ ์„ค์ • ๊ฐ€๋Šฅํ•ฉ๋‹ˆ๋‹ค.",
textAlign = TextAlign.Start,
style = CchTheme.typography.bodyMd,
color = Red300,
)
}
Comment on lines +113 to +123

@coderabbitai coderabbitai Bot Aug 26, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐Ÿงน Nitpick (assertive)

๋ฌธ๊ตฌ ํ•˜๋“œ์ฝ”๋”ฉ โ†’ ๋ฆฌ์†Œ์Šคํ™” ๊ณ ๋ ค

๊ตญ์ œํ™”/๋ณต์ˆ˜ ํ”Œ๋žซํผ ๋Œ€์‘์„ ์œ„ํ•ด ์—๋Ÿฌ ๋ฉ”์‹œ์ง€๋ฅผ String ๋ฆฌ์†Œ์Šค๋กœ ์ด๋™ํ•˜๊ณ  ์ž๋ฆฌํ‘œ์‹œ์ž์— ์ƒ์ˆ˜๋ฅผ ์ฃผ์ž…ํ•˜๋Š” ๋ฐฉ์‹์„ ๊ถŒ์žฅํ•ฉ๋‹ˆ๋‹ค.

์˜ˆ)

  • strings: timetable_name_over_limit = "์‹œ๊ฐ„ํ‘œ ์ด๋ฆ„์€ ์ตœ๋Œ€ %1$d์ž๊นŒ์ง€ ์„ค์ • ๊ฐ€๋Šฅํ•ฉ๋‹ˆ๋‹ค."
  • ์ฝ”๋“œ: text = stringResource(Res.string.timetable_name_over_limit, TIMETABLE_NAME_LIMIT)

ํ˜„์žฌ PR ๋ฒ”์œ„๋ฅผ ์ตœ์†Œํ™”ํ•˜๋ ค๋ฉด ์œ„์˜ ์ƒ์ˆ˜ ๋ณด๊ฐ„ ๋ฒ„์ „์œผ๋กœ ๋ฐ˜์˜ํ•˜๊ณ , ํ›„์† PR์—์„œ ๋ฆฌ์†Œ์Šคํ™”๋ฅผ ์ง„ํ–‰ํ•ด๋„ ๋ฉ๋‹ˆ๋‹ค.

๐Ÿค– Prompt for AI Agents 
In
composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/presentation/timetable/timetablenameinput/TimetableNameInputScreen.kt
around lines 113โ€“123, replace the hardcoded Korean error text with a
stringResource that injects the max-length constant; change text = "์‹œ๊ฐ„ํ‘œ ์ด๋ฆ„์€ ์ตœ๋Œ€
20์ž๊นŒ์ง€ ์„ค์ • ๊ฐ€๋Šฅํ•ฉ๋‹ˆ๋‹ค." to text = stringResource(Res.string.timetable_name_over_limit,
TIMETABLE_NAME_LIMIT) (or similar project resource identifier), ensure
TIMETABLE_NAME_LIMIT constant is referenced/defined and import
stringResource/Res.string as needed so the message uses the constant placeholder
rather than hardcoded "20".


Spacer(modifier = Modifier.weight(1f))

CchBasicButton(
Expand Down
Loading