파일 및 공통 모듈 리팩토링 #73
파일 및 공통 모듈 리팩토링 #73
Conversation
- 색상 리스트 열 때, 바텀 시트가 내려가는 에러 수정 - sheetState: SheetState = rememberModalBottomSheetState() 매개변수 추가해서 오류 픽스 및 각 바텀 시트에서 상태를 수정할 수 있도록 수정 - TextFieldFileBottomSheet: modifier.padding(top = 14.dp) 위치를 AnimatedVisibility에서 내부의 VerticalGrid로 이동
- FileScreen()을 FileApp()으로 변경
- `gradle/libs.versions.toml`에 `foundationLayout` 버전 및 라이브러리 정의 추가 - `feature/file` 모듈의 `build.gradle.kts`에 `androidx-compose-foundation-layout` 의존성 추가
- `enabled`, `onClickLabel`, `role` 파라미터를 추가하여 `clickable` 기능 확장 및 접근성 지원 - KDoc 주석을 추가하여 함수 및 각 파라미터의 역할 명시 - 내부 `clickable` 구현에서 고정되어 있던 인자들을 전달받은 파라미터로 동작하도록 수정
- `gradientTint` 함수의 파라미터 및 동작 방식에 대한 상세 설명 추가 - `graphicsLayer`와 `drawWithCache`를 사용한 구현 방식 및 `BlendMode.SrcIn` 기본값에 대한 설명 명시
- `BottomFolderEditBottomSheet`에서 `TextFieldFileBottomSheet` 호출 시 `sheetState` 인자를 추가하여 상태 관리 개선 - `rememberModalBottomSheetState`에 `skipPartiallyExpanded = true` 옵션을 적용하여 바텀 시트가 항상 전체 확장 상태로 열리도록 수정 - 불필요한 `FileViewModel` 임포트 제거
- `linkList.isNotEmpty()` 조건을 추가하여 "분류되지 않은 링크" 텍스트와 링크 그리드가 데이터가 있을 때만 표시되도록 수정
- 새로운 메뉴 체크 아이콘 에셋(`ic_top_folders_menu.xml`) 추가 - `TopFolderListMenu` 비즈니스 로직과 UI 레이아웃(`TopFolderListMenuLayout`) 분리 - 메뉴의 각 항목을 담당하는 `TopFolderListMenuRow` 컴포저블 추가 - `noRippleClickable`, `gradientTint` 등 커스텀 Modifier를 적용하여 코드 간결화 및 재사용성 향상 - 각 컴포저블 및 파라미터에 대한 상세 KDoc 주석 추가 - 드롭다운 메뉴 너비(150dp -> 180dp) 및 내부 패딩 등 세부 UI 스타일 수정
- **기능 추가 및 개선**
- 검색 결과가 없을 때 표시할 안내 문구 및 아이콘(`ic_search_bar_caution`) 추가
- 검색어 입력창에 텍스트가 있을 때만 삭제(`X`) 버튼이 보이도록 노출 로직 수정
- 탑 시트가 닫힐 때(`visible == false`), 뒤로가기 버튼 클릭, 배경 딤(Dim) 클릭 시 입력 텍스트 및 수정 모드 상태를 초기화하도록 개선
- 검색어 입력 시 `collectLatest`를 사용하여 불필요한 이전 검색 요청을 취소하도록 로직 최적화
- **UI/UX 및 스타일 수정**
- 최근 검색어의 삭제 버튼 아이콘을 전용 리소스(`ic_recent_search_x`)로 교체 및 색상 조정
- 뒤로가기 아이콘을 기본 Material Vector에서 커스텀 리소스(`ic_back`)로 변경
- 최근 검색어 영역의 "최근 검색" 타이틀 폰트 두께를 `Bold`로 변경하고 간격 및 패딩 미세 조정
- `HighlightedText` 컴포넌트의 가독성을 위해 정규식 처리 및 스타일 적용 로직 정리
- **코드 리팩터링 및 가독성**
- 컴포넌트 및 파라미터에 상세 KDoc 주석 추가
- `RecentQueryItem` -> `RecentQueryChip`, `FastSearchItem` -> `FastSearchItemRow` 등 컴포저블 함수명 직관화
- 변수명 오타 수정 (`recentQuerys` -> `recentQueries`) 및 중복 코드 함수화(`resetAndDismiss`)
- **리소스 추가**
- `ic_recent_search_x.xml`, `ic_search_bar_caution.xml` 벡터 드로어블 추가
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Walkthrough이 PR은 검색 UI와 폴더 편집 바텀시트 기능을 개선합니다. SearchBarTopSheet의 매개변수를 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
feature/file/src/main/java/com/example/file/ui/bottom/sheet/TopFolderEditBottomSheet.kt (1)
25-31: Non-null assertion (!!) 사용으로 인한 잠재적 NPE 위험
folderStateViewModel.readyToUpdateTopFolder!!.folderName에서!!연산자를 사용하고 있습니다.onColorIdDeliver콜백이 실행되는 시점에readyToUpdateTopFolder가 null일 경우 앱이 크래시됩니다.안전한 처리를 위해 null 체크를 추가하거나 early return 패턴을 사용하는 것을 권장합니다.
🔧 안전한 null 처리 제안
onColorIdDeliver = { colorId -> + val topFolder = folderStateViewModel.readyToUpdateTopFolder ?: return@TextFieldFileBottomSheet fileViewModel.updateCategoryColor( - categoryName = folderStateViewModel.readyToUpdateTopFolder!!.folderName, + categoryName = topFolder.folderName, colorId = (colorId + 1).toLong(), colorStyle = CategoryColorStyle.categoryStyleList[colorId] ) },feature/file/src/main/java/com/example/file/ui/bottom/sheet/NewBottomFolderBottomSheet.kt (1)
13-20:NewBottomFolderBottomSheet에서sheetState파라미터 누락 - 다른 폴더 관련 바텀 시트와 일관성 부족
BottomFolderEditBottomSheet,TopFolderEditBottomSheet,ShareBottomSheet,LinkCategorizationBottomSheet는 모두sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true)를 명시적으로 전달하여 바텀 시트가 부분 확장 상태를 건너뛰고 바로 전체 확장되도록 합니다.
NewBottomFolderBottomSheet는sheetState를 전달하지 않아TextFieldFileBottomSheet의 기본값(skipPartiallyExpanded = false)을 사용하므로, 이 컴포넌트만 부분 확장 상태를 허용합니다.동일한 목적의 다른 폴더 관련 바텀 시트들과 동작을 맞추기 위해
sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true)를 전달하는 것을 권장합니다.
🤖 Fix all issues with AI agents
In `@design/src/main/java/com/example/design/top/search/SearchBarTopSheet.kt`:
- Around line 149-156: The debounce coroutine restarts because
LaunchedEffect(text) relaunches on every text change; change it to
LaunchedEffect(Unit) so the coroutine starts once and uses snapshotFlow to
observe text changes, keeping the existing snapshotFlow { text }. Ensure you
keep the chain .map { it.trim() }.filter { it.length >= 2
}.debounce(350).distinctUntilChanged().collectLatest(onQueryChange) and remove
the text key from LaunchedEffect so debounce works as intended with
onQueryChange.
- Around line 123-128: The LaunchedEffect(visible) in SearchBarTopSheet calls
resetAndDismiss() when visible becomes false, which can cause onDismiss() to be
invoked twice; change the behavior so the effect only resets internal state and
does not call onDismiss(): either remove or refactor the onDismiss() call out of
resetAndDismiss() (e.g., create a dedicated resetState() used by the
LaunchedEffect) and keep onDismiss() invocation only at the external dismiss
entrypoint; update calls to use resetState()/resetAndDismiss() accordingly and
retain names resetAndDismiss() and onDismiss() for clarity.
In `@feature/file/build.gradle.kts`:
- Line 56: The project mixes Compose BOM and an explicit version for
foundation.layout; decide one approach and make it consistent: either remove the
explicit version reference for foundation.layout (remove
version.ref="foundationLayout" on
implementation(libs.androidx.compose.foundation.layout) so the
androidx.compose.bom manages it) or remove the androidx.compose.bom dependency
and keep the explicit foundation.layout version; update the implementation entry
for foundation.layout or the BOM declaration accordingly and ensure
imports/usages (e.g., Box, Column, Row, Arrangement, padding) continue to
compile.
In
`@feature/file/src/main/java/com/example/file/ui/top/bar/component/TopFolderListMenu.kt`:
- Around line 197-201: The clickable menu item using noRippleClickable (where
enabled = selectedOption != selectedText and onClick = onClick) lacks an
explicit accessibility role; update the call to pass the semantics role (e.g.,
Role.Button) — i.e., add the role parameter to noRippleClickable so TalkBack
users receive the proper element role (you can also include onClickLabel if
desired for a more descriptive announcement).
🧹 Nitpick comments (9)
gradle/libs.versions.toml (1)
34-34: foundation과 foundation-layout 버전 불일치.
foundation버전(1.9.5, Line 33)과foundationLayout버전(1.10.0)이 다릅니다. 두 라이브러리는 같은 Compose Foundation 모듈 계열이므로 버전이 일치해야 호환성 문제를 방지할 수 있습니다.Compose BOM(
composeBom = "2024.09.00")을 사용 중이라면, BOM이 버전을 자동으로 관리하도록 개별 버전 지정 없이 사용하는 것을 권장합니다.♻️ BOM 사용 시 권장 수정안
BOM을 통해 버전을 관리하려면:
-androidx-compose-foundation-layout = { group = "androidx.compose.foundation", name = "foundation-layout", version.ref = "foundationLayout" } +androidx-compose-foundation-layout = { group = "androidx.compose.foundation", name = "foundation-layout" }또는 명시적 버전을 유지하려면 foundation과 동일한 버전으로 통일:
-foundationLayout = "1.10.0" +foundationLayout = "1.9.5"design/src/main/java/com/example/design/modifier/GradientTint.kt (1)
20-26: 중복된 주석을 제거하세요.새로 추가된 KDoc(lines 9-19)이 이미 파라미터들을 충분히 설명하고 있으므로, 함수 내부의 인라인 주석(lines 21-24)은 중복입니다.
♻️ 제안하는 수정
fun Modifier.gradientTint( - /* - * brush: 그라데이션 색상을 적용할 브러시 - * blendMode: 그라데이션 색상을 적용할 때 사용할 블렌드 모드 - * */ brush: Brush, blendMode: BlendMode = BlendMode.SrcIn ): Modifier = this.graphicsLayer(alpha = 0.99f)design/src/main/java/com/example/design/top/search/SearchBarTopSheet.kt (5)
130-139: 주석 처리된 코드를 제거하세요.이전 구현이 주석 처리되어 있습니다. 버전 관리 시스템에서 히스토리를 추적할 수 있으므로, 사용하지 않는 코드는 삭제하는 것이 좋습니다.
348-354:resetAndDismiss()함수를 일관되게 사용하세요.배경 클릭 핸들러와 뒤로가기 버튼(lines 411-414)에서 동일한 로직(
text = "",isEditMode = false,onDismiss())이 중복됩니다.resetAndDismiss()함수가 이미 정의되어 있으므로 이를 활용하세요.단, 앞서 언급한
LaunchedEffect(visible)수정을 적용한 후에 사용해야 합니다.♻️ 제안하는 수정 (배경 클릭)
.noRippleClickable { - text = "" - isEditMode = false - onDismiss() + resetAndDismiss() }
300-301: 사용되지 않는uriHandler변수.
uriHandler가 선언되었지만 사용되지 않습니다. 불필요한 변수는 제거하세요.♻️ 제안하는 수정
- // uri 핸들러 - val uriHandler = LocalUriHandler.current
LocalUriHandlerimport도 함께 제거하세요.
247-248: Regex 객체 생성 최적화를 고려하세요.
Regex객체가 매 리컴포지션마다 생성됩니다.searchTerm이 변경될 때만 재생성하도록remember를 사용할 수 있습니다.♻️ 제안하는 수정
+ val regex = remember(searchTerm) { + Regex("(?i)${Regex.escape(searchTerm)}") + } - val regex = Regex("(?i)${Regex.escape(searchTerm)}")단,
HighlightedText가 내부 컴포저블이므로 영향은 제한적입니다.
306-309: 에러 처리 로직 검토 필요.
runCatching의onFailure블록이 비어 있어 오류가 무시됩니다.onLinkClick은 콜백이므로 예외 발생 가능성이 낮지만, 필요시 로깅을 추가하거나runCatching을 제거하는 것을 고려하세요.feature/file/src/main/java/com/example/file/ui/top/bar/component/TopFolderListMenu.kt (2)
68-101: 상태 업데이트를 단일 ViewModel 함수로 묶어 일관성 확보
여러update*호출이 분산돼 있어 중간 상태가 노출되거나 불필요한 재구성이 발생할 수 있습니다. 선택 로직과getSharedFolders()호출을 ViewModel의 단일 메서드로 캡슐화해 원자적으로 처리하는 쪽을 권장합니다.
129-163: 하드코딩 문자열을 string resource로 분리 권장
"나의 폴더","공유받은 폴더"가 코드에 직접 박혀 있어 i18n 및 UI 테스트에 불리합니다.stringResource(R.string...)로 분리하면 관리가 쉬워집니다.
| // 체크가 안된 항목만 체크 가능 | ||
| .noRippleClickable( | ||
| enabled = selectedOption != selectedText, | ||
| onClick = onClick | ||
| ), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "fun\s+noRippleClickable\b" -S --context 10Repository: LinkYou-2025/LinkU_Android
Length of output: 52
🏁 Script executed:
rg "noRippleClickable" -t kt --context 10Repository: LinkYou-2025/LinkU_Android
Length of output: 96
🏁 Script executed:
rg "noRippleClickable" --type-list | grep -i kotlinRepository: LinkYou-2025/LinkU_Android
Length of output: 459
🏁 Script executed:
rg "noRippleClickable" -A 15 --color neverRepository: LinkYou-2025/LinkU_Android
Length of output: 50382
🏁 Script executed:
cat design/src/main/java/com/example/design/modifier/NoRippleClickable.ktRepository: LinkYou-2025/LinkU_Android
Length of output: 1353
🏁 Script executed:
sed -n '190,210p' feature/file/src/main/java/com/example/file/ui/top/bar/component/TopFolderListMenu.ktRepository: LinkYou-2025/LinkU_Android
Length of output: 499
noRippleClickable 사용 시 명시적 접근성 Semantics 추가 권장
noRippleClickable 구현부는 role, onClickLabel 등 접근성 semantics를 올바르게 보존하고 있습니다. 다만 이 메뉴 항목(lines 197-201)에서는 role 파라미터가 명시되지 않아, TalkBack 사용자에게 요소의 역할이 명확하게 전달되지 않을 수 있습니다.
.noRippleClickable(enabled = selectedOption != selectedText, role = Role.Button, onClick = onClick)
와 같이 role 파라미터를 추가하여 더 나은 접근성 경험을 제공하시기 바랍니다.
🤖 Prompt for AI Agents
In
`@feature/file/src/main/java/com/example/file/ui/top/bar/component/TopFolderListMenu.kt`
around lines 197 - 201, The clickable menu item using noRippleClickable (where
enabled = selectedOption != selectedText and onClick = onClick) lacks an
explicit accessibility role; update the call to pass the semantics role (e.g.,
Role.Button) — i.e., add the role parameter to noRippleClickable so TalkBack
users receive the proper element role (you can also include onClickLabel if
desired for a more descriptive announcement).
gradle/libs.versions.toml
Outdated
| androidx-compose-foundation = { group = "androidx.compose.foundation", name = "foundation", version.ref = "foundation" } | ||
| androidx-compose-testing = { group = "androidx.xr.compose", name = "compose-testing", version.ref = "composeTesting" } | ||
| androidx-tools-core = { group = "androidx.privacysandbox.tools", name = "tools-core", version.ref = "toolsCore" } | ||
| androidx-compose-foundation-layout = { group = "androidx.compose.foundation", name = "foundation-layout", version.ref = "foundationLayout" } |
There was a problem hiding this comment.
git 충돌이 있어서, 아침에 이 부분만 수정해서 아침에 커밋 푸쉬했습니다.
| enabled: Boolean = true, | ||
| onClickLabel: String? = null, | ||
| role: Role? = null, | ||
| onClick: () -> Unit |
There was a problem hiding this comment.
여기 변경된 파라미터에 맞춰서 수정하겠습니다.
|
|
||
| // 입력 변화 디바운스 수집 (2자 이상 + 350ms) | ||
| // - mapLatest: 새 입력이 오면 이전 요청(코루틴 Job) 자동 취소 → 레이스 방지 | ||
| // 닫힐 때 상태 초기화 로직 (visible이 false가 될 때) |
There was a problem hiding this comment.
여기에 사용자가 탑 시트를 열었을 때, 자동으로 검색창에 커서가 가고 키보드 올라오도록 FocusRequester를 이용하도록 리펙토링 해보시는건 어떨까요?
| LaunchedEffect(text) { | ||
| snapshotFlow { text } | ||
| .map { it.trim() } | ||
| .filter { it.length >= 2 } |
There was a problem hiding this comment.
검색했을 때, 한 글자만 검색한 사용자 입장에서 왜 검색이 안되지? 싶을 수 있어서, 디자이너와 상의 후 관련 ui 추가하는 건 어떠실까요?
There was a problem hiding this comment.
검색했을 때, 한 글자만 검색한 사용자 입장에서 왜 검색이 안되지? 싶을 수 있어서, 디자이너와 상의 후 관련 ui 추가하는 건 어떠실까요?
윤다인과 이다현의 TODO. 한글자일 때 비활성화+상태메시지 ui
| onSelectSharedFolders: () -> Unit | ||
| ){ | ||
| // 선택된 폴더 | ||
| val selectedText = if (isSharedFolders) "공유받은 폴더" else "나의 폴더" |
There was a problem hiding this comment.
enum으로 분리하는게 깔끔할 것 같아용
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
design/src/main/java/com/example/design/top/search/SearchBarTopSheet.kt (1)
400-414: 클릭 가능한 아이콘에 접근성 라벨을 추가해주세요.뒤로가기 및 검색어 지우기 버튼이
.noRippleClickable등으로 클릭 가능하지만contentDescription = null로 설정되어 있어 스크린리더에서 읽히지 않습니다. Jetpack Compose 접근성 가이드라인에 따르면, 클릭 가능한 Icon/Image 요소는 반드시 의미있는contentDescription을 제공해야 합니다.제안 변경
Icon( modifier = Modifier .height(40.dp) .noRippleClickable (onClick = onDismiss), tint = colors.gray[600], painter = painterResource(id = R.drawable.ic_back), - contentDescription = null + contentDescription = "뒤로가기" )if(text.isNotEmpty()){ Image( modifier = Modifier .padding(end = 18.dp) .size(18.dp) .noRippleClickable { text = "" }, painter = painterResource(R.drawable.ic_text_clear), - contentDescription = null + contentDescription = "검색어 지우기" ) }
🧹 Nitpick comments (2)
design/src/main/java/com/example/design/top/search/SearchBarTopSheet.kt (1)
129-138: 주석 처리된 이전 디바운스 블록은 제거하는 편이 좋습니다.
현재 구현이 있으므로 주석 블록은 혼동만 유발합니다.🧹 제안 변경
-/* - LaunchedEffect(Unit) { - snapshotFlow { text } - .map { it.trim() } - .filter { it.length >= 2 } // 2자 이상일 때만 - .debounce(350) // 350ms 주기 탐색 - .distinctUntilChanged() - .mapLatest(onQueryChange) - .collect { /* no-op */ } - }*/feature/home/src/main/java/com/example/home/screen/HomeScreen.kt (1)
203-205: itemsToRender/titleText 중복 선언과 하드코딩 이름은 정리 권장합니다.
바깥 titleText가 “세나님”으로 고정되고 내부에서 다시 선언되어 혼동 여지가 있습니다. 하나만 유지하고 userName 기반으로 통일하세요.♻️ 제안 변경
- val titleText = if (showRecs) "세나님의 오늘에 어울리는 콘텐츠예요!" + val titleText = if (showRecs) "${userName}님의 오늘에 어울리는 콘텐츠예요!" else "${userName}님이 최근에 열람한 링크" @@ - val itemsToRender = if (showRecs) recommendedLinks else recentLinks - val titleText = if (showRecs) "${userName}님의 오늘에 어울리는 콘텐츠예요!" - else "${userName}님이 최근에 열람한 링크"Also applies to: 257-264
ugmin1030
force-pushed
the
feature/#44
branch
from
c2f8ac9 to
9834a5c
Compare
- `feature/file/build.gradle.kts`에서 `androidx.compose.foundation.layout` 의존성을 `androidx.compose.foundation`으로 변경
- `LinksGrid.kt` 내 삭제 확인 모달의 버튼 텍스트를 "삭제" -> "삭제하기", "취소" -> "취소하기"로 변경 - 미사용 임포트(`lifecycleScope`, `rememberCoroutineScope`, `launch` 등) 제거 및 임포트 최적화
- `FileBottomSheet`에 0.5 투명도의 블랙 딤 컬러(`scrimColor`) 적용 - 텍스트 간격 조정을 위해 `Spacer` 추가 및 불필요한 `lineHeight`, `padding` 제거
- `animateFloatAsState`의 `label` 파라미터에 "화살표 회전 애니메이션" 명시 및 `targetValue` 네임드 파라미터 적용
- `LinkItemLayout`에 그림자 효과(`shadow`)를 적용하고 기존 `shadowElevation` 속성 제거 - 태그 UI 레이아웃 수정: `padding` 값 조정, 폰트 크기 변경(10sp -> 12sp), `includeFontPadding = false` 적용 - 컴포넌트 간 간격 조정을 위해 `Spacer` 추가 및 기존 `padding` 로직 수정 - 불필요한 `showDialog` 상태 및 미사용 import 제거 - 클릭 이벤트 로직 내 불필요한 null safety 체크 최적화
- `selectedLinks`를 `mutableStateListOf`로 변경하여 상태 추적 지원 - 바텀시트 '추가' 버튼 활성화 상태(`isReady`) 조건 추가 (선택된 링크가 있을 때만 활성화) - 링크 아이템 레이아웃 수정: `offset`, `padding`, `shape` 및 정렬 조정 - 체크박스 미선택 시 색상(`uncheckedColor`) 변경 - 링크 선택 시 데이터 정합성을 위해 `onOkay` 콜백 내 `selectedLinks.clear()` 로직 추가
- `ShareFolderIcon`, `LockFolderIcon`, `PencilIcon`, `BookMarkStar` 함수에 `internal` 접근 제한자 적용
📝 설명
✔️ PR 유형
어떤 변경 사항이 있나요?
📎 관련 이슈 번호
Summary by CodeRabbit
릴리스 노트
새로운 기능
스타일
버그 수정