[Feature]: 폴더 UI 사용 라이브러리 에러 해결#145
Conversation
- `SelectDepth` 인터페이스에 `Comparable` 구현 및 `depthOrder` 프로퍼티 추가 - `None`, `Category`, `Folder` 각 상태에 따른 깊이 순서값(depthOrder) 정의 - `Folder` 클래스가 `Category`를 상속받도록 구조 변경 및 데이터 클래스에서 일반 클래스로 전환
- 기존 외부 라이브러리의 `VerticalGrid`를 Jetpack Compose 표준 `LazyVerticalGrid`로 마이그레이션 - `LinksGrid` 컴포저블에 `modifier` 및 `contentPadding` 파라미터를 추가하여 외부 레이아웃 제어 지원 - 기존 `for` 루프 방식을 `LazyVerticalGrid` 내 `item` 및 `itemsIndexed` 사용 구조로 변경 - 아이템 정렬 방식을 `Box`와 `Alignment` 조합에서 `Row`와 `Arrangement` 조합으로 변경하여 배치 구조 개선 - `LinksGridTest`에서 ViewModel 파라미터 이름을 명시적으로 선언하도록 수정
- `FolderGrid`에서 `onFolderClick` 콜백 매개변수를 제거하고, 클릭 이벤트 처리를 `folderLayout` 내부 아이템으로 위임 - `TopFolderGrid`에서 `TopFolderItemLayout`에 직접 `noRippleClickable` 수식어를 적용하여 클릭 이벤트 및 비즈니스 로직을 처리하도록 변경 - `FolderGrid` 매개변수 `FolderLayout` 명칭을 카멜케이스 규칙에 맞춰 `folderLayout`으로 수정 - `FolderGrid` 내부의 그리드 간격 수치(18.51)를 `MIDDLE_PADDING` 상수로 분리 정의
- `LazyVerticalGrid` 내부에 직접 구현되어 있던 그리드 아이템 렌더링 로직을 `LazyGridScope`의 확장 함수(`LinksGrid`, `FolderGrid`)로 분리하여 재사용성 강화 - "링크 추가하기" 아이템을 별도의 private `@Composable` 컴포넌트(`AddLinkItem`)로 추출 - 그리드 아이템 배치 정렬(좌/우) 시 오프셋을 반영할 수 있도록 `itemIndexOffset` 파라미터 추가 및 정렬 로직 개선 - 클릭 및 롱클릭 이벤트를 상위에서 주입받을 수 있도록 람다 콜백 매개변수화 및 구조 변경
Walkthrough파일 화면의 상단·중단·하단 그리드가 새 컴포저블로 교체되고, 선택 깊이 모델이 비교 가능 구조로 바뀌었습니다. 폴더/링크 카드 레이아웃은 새 이름과 크기 조정 방식으로 정리되었고, 공유 바텀시트도 새 폴더 아이템 레이아웃을 사용하도록 갱신되었습니다. Changes폴더·링크 UI 재구성
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the folder and link grid layouts to use LazyVerticalGrid and extracts reusable LazyGridScope extension functions. It also introduces a comparable hierarchy for SelectDepth. However, having Folder inherit from Category (making Category an open class) loses critical data class benefits and introduces a severe UI bug where Folder checks become unreachable in when-expressions. Additionally, missing keys in itemsIndexed can cause performance issues, and calling ViewModels directly inside @Preview will break the layout preview rendering.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| private class Folder(override val category: FolderSimpleInfo, val folder: FolderSimpleInfo) : Category(category) { | ||
| override val depthOrder = 2 | ||
| } |
There was a problem hiding this comment.
Folder가 Category를 상속받도록 변경되면서 심각한 UI 버그가 발생합니다.
Folder가 Category의 하위 클래스가 됨에 따라, selectDepth is Category 검사는 Folder 인스턴스에 대해서도 true를 반환하게 됩니다.
이로 인해 menuText를 생성하는 when (selectDepth) 블록(405~421라인)에서 is Category 분기가 is Folder 분기보다 먼저 위치해 있어, selectDepth가 Folder일 때도 is Category 분기를 타게 되어 is Folder 블록은 **도달할 수 없는 코드(Unreachable Code)**가 됩니다. 결과적으로 폴더를 선택해도 UI에는 카테고리 이름만 표시되는 버그가 발생합니다.
또한, 상속 대신 인터페이스 구현을 통한 구성(Composition)을 사용하면 리스코프 치환 원칙(LSP)을 준수하고 data class 고유의 이점을 유지할 수 있습니다.
| private class Folder(override val category: FolderSimpleInfo, val folder: FolderSimpleInfo) : Category(category) { | |
| override val depthOrder = 2 | |
| } | |
| private data class Folder(val category: FolderSimpleInfo, val folder: FolderSimpleInfo) : SelectDepth { | |
| override val depthOrder = 2 | |
| } |
There was a problem hiding this comment.
[Q, P2] 이번 PR 범위는 아닌데 화면 비율 계산 관련해서 질문 드립니다.
private const val FULL_HEIGHT = 917f
private const val SHEET_MAIN_SCREEN_HEIGHT_RATIO = SHEET_MAIN_SCREEN_HEIGHT / FULL_HEIGHT
FULL_HEIGHT가 Figma 디자인 시안의 고정 기준값(412×917)으로 보이는데, 실제 디바이스의
화면 높이를 읽어오는 코드는 안 보입니다. fillMaxHeight(ratio)는 부모 컴포저블의
실측 높이 × ratio로 동작하니, 디바이스마다 곱해지는 베이스 값은 다르지만 비율
자체는 412×917 화면을 기준으로 고정된 값입니다.
이 비율이 다른 종횡비의 화면(더 길쭉한 화면, 더 짧은 화면, 태블릿 등)에도 그대로
곱해지면, 디자인 시안에서 의도했던 여백감/비율감과 달라질 수 있을 것 같습니다.
비율 스케일링이 등비가 아니라 디자인 시안 자체의 가로세로 비율에 종속된 값이라서요.
혹시 의도적으로 이렇게 두신 건지, 아니면 scaler 유틸(Figma 412×917 baseline)을
이 화면에는 적용 안 하신 이유가 있으신지 궁금합니다. scaler를 쓰면 LocalConfiguration이나
BoxWithConstraints로 실제 화면 높이를 받아서 디자인 기준 dp 값을 직접 변환하는
방식이었던 것 같은데, 이 파일에서는 비율(fraction)을 직접 계산해서 fillMaxHeight에
넣는 방식으로 다르게 가셔서 — 두 방식을 통일하는 게 나을지 의견 여쭤봅니다.
물론 scaler도 수정할 점이 있을거라, 아예 이거 수정해서 쓰는게 더 좋지 않을까 싶습니다~!
- 제 생각을 ai에게 정리를 부탁드렸습니다:) -
There was a problem hiding this comment.
[p2] 이번 PR은 아닌데 colors: ThemeColorScheme를 매 컴포저블마다 파라미터로 넘기는 패턴 관련해서
의견 드립니다.
val colors = MaterialTheme.linkuColors
이걸 ShareBottomSheetLayout 최상위에서 한 번 받아서, 5~6단계 자식 컴포저블까지
계속 파라미터로 손으로 넘기고 있습니다 (ShareBottomSheetLayout →
ShareBottomSheetMainScreen → LinkShareButton/LinkCopyButton/FolderStructure/
ShareFolderMenu/ShareLink/LinkGenerationButton).
MaterialTheme.linkuColors는 CompositionLocal인데, CompositionLocal의 핵심 가치가
"트리 어디서든 직접 접근 가능해서 prop drilling을 피한다"는 거라서, 지금처럼 매번
파라미터로 넘기면 그 가치를 못 살리고 일반 함수 파라미터처럼 쓰게 되는 것 같습니다.
이렇게 되면:
- 모든 leaf 컴포저블 시그니처에 colors가 박혀서, 함수만 봤을 때 "이게 진짜 필요한
의존성인가"가 헷갈립니다. - Preview 작성 시 colors = MaterialTheme.linkuColors를 매번 또 호출해줘야 합니다.
이미 ThemeProvider 안에 있다면 내부에서 직접 읽으면 될 것 같습니다. - 다른 화면에서 LinkShareButton 같은 컴포넌트를 재사용하려면 호출하는 쪽이 colors를
매번 구해서 넘겨줘야 합니다.
leaf 컴포저블들(LinkShareButton, LinkCopyButton, FolderStructure, ShareFolderMenu,
ShareLink, LinkGenerationButton)에서는 colors 파라미터를 빼고 내부에서 직접
MaterialTheme.linkuColors를 읽는 방향이 어떨지 제안 드립니다.
@composable
private fun LinkShareButton(
modifier: Modifier,
onShare: () -> Unit
) {
val colors = MaterialTheme.linkuColors
// ...
}
다만 테스트에서 다른 색상 스킴을 주입하고 싶거나, 같은 컴포저블이 여러 다른 테마
컨텍스트에서 동시에 쓰일 수 있는 경우라면 파라미터로 넘기는 게 합리적일 수 있는데,
지금 코드에서는 그런 이유가 보이지 않아서 여쭤봅니다. 혹시 의도하신 부분이 있을까요?
| private open class Category(open val category: FolderSimpleInfo) : SelectDepth{ | ||
| override val depthOrder = 1 | ||
| } |
There was a problem hiding this comment.
Category를 open class로 변경하면서 data class가 제공하는 equals(), hashCode(), toString(), copy() 등의 자동 생성 메서드를 잃게 되었습니다.
특히 Compose에서는 remember(selectDepth) (403라인)와 같이 상태 변화를 감지할 때 값 비교(equals)를 자주 사용하므로, data class가 아닐 경우 불필요한 리컴포지션이 발생하거나 상태 변경이 올바르게 감지되지 않을 수 있습니다.
Category와 Folder 간의 상속 관계를 제거하고 각각 독립적인 data class로 정의하여 SelectDepth 인터페이스를 구현하도록 변경하는 것을 권장합니다.
| private open class Category(open val category: FolderSimpleInfo) : SelectDepth{ | |
| override val depthOrder = 1 | |
| } | |
| private data class Category(val category: FolderSimpleInfo) : SelectDepth { | |
| override val depthOrder = 1 | |
| } |
There was a problem hiding this comment.
[p3] 참고로 object보다 data object를 쓰면 toString()도 깔끔하게 나온다고 합니다 -클로드-
| itemIndexOffset: Int = 0, | ||
| folderLayout: @Composable (folder: FolderSimpleInfo, colorStyle: CategoryColorStyle) -> Unit | ||
| ) { | ||
| itemsIndexed(folderList) { index, folder -> |
There was a problem hiding this comment.
LazyVerticalGrid나 LazyColumn과 같은 Lazy 레이아웃에서 itemsIndexed를 사용할 때 고유한 key를 제공하지 않으면, 리스트의 아이템이 추가/삭제/재정렬될 때 전체 아이템이 불필요하게 리컴포지션되거나 스크롤 상태가 유실될 수 있습니다.
FolderSimpleInfo가 가지고 있는 고유 ID인 folderId를 key로 지정하여 성능과 렌더링 안정성을 향상시키는 것을 권장합니다.
itemsIndexed(
items = folderList,
key = { _, folder -> folder.folderId }
) { index, folder ->| } | ||
| } | ||
|
|
||
| itemsIndexed(linkList) { index, link -> |
There was a problem hiding this comment.
LazyVerticalGrid에서 itemsIndexed를 사용할 때 고유한 key를 제공하는 것이 좋습니다. LinkItemInfo가 가진 고유 ID인 linkuId를 key로 지정하면, 리스트 변경 시 불필요한 리컴포지션을 방지하고 스크롤 상태를 안정적으로 유지할 수 있습니다.
itemsIndexed(
items = linkList,
key = { _, link -> link.linkuId }
) { index, link ->| @Preview(showBackground = true) | ||
| @Composable | ||
| private fun LinksGridTest(){ | ||
| LinksGrid( | ||
| hiltViewModel(), | ||
| viewModel(), | ||
| fileViewModel = hiltViewModel(), | ||
| folderStateViewModel = viewModel(), | ||
| ) | ||
| } No newline at end of file | ||
| } |
There was a problem hiding this comment.
@Preview 환경에서 hiltViewModel()이나 viewModel()을 직접 호출하여 실제 ViewModel 인스턴스를 생성하려고 하면, 의존성 주입 컨텍스트나 ViewModelStoreOwner가 올바르게 구성되지 않아 Android Studio의 Preview 렌더링이 실패하거나 런타임 에러가 발생할 수 있습니다.
이미 stateless하게 잘 분리된 LazyGridScope.LinksGrid 확장 함수를 활용하여, Preview 내에서 직접 LazyVerticalGrid를 구성하고 더미 데이터를 전달하도록 수정하면 Preview가 정상적으로 렌더링됩니다.
| @Preview(showBackground = true) | |
| @Composable | |
| private fun LinksGridTest(){ | |
| LinksGrid( | |
| hiltViewModel(), | |
| viewModel(), | |
| fileViewModel = hiltViewModel(), | |
| folderStateViewModel = viewModel(), | |
| ) | |
| } | |
| \ No newline at end of file | |
| } | |
| @Preview(showBackground = true) | |
| @Composable | |
| private fun LinksGridTest(){ | |
| LazyVerticalGrid( | |
| columns = GridCells.Fixed(2), | |
| contentPadding = PaddingValues(20.dp) | |
| ) { | |
| this.LinksGrid( | |
| linkList = emptyList(), | |
| isShareMode = false, | |
| onAddLinkClick = {}, | |
| onLinkClick = {}, | |
| onLinkLongClick = {} | |
| ) | |
| } | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/file/src/main/java/com/linku/file/ui/bottom/sheet/_ShareBottomSheet.kt (1)
177-190: 🎯 Functional Correctness | 🟠 Major
remember(selectDepth)의존성 추적이 실패하여 UI 상태가 갱신되지 않을 수 있습니다.줄 403의
remember(selectDepth){ mutableStateOf(...) }에서selectDepth를 Compose의 의존성 키로 사용하고 있습니다. Compose는 이 키가 변경되었는지 확인하기 위해 내부적으로equals()를 호출합니다. 그런데Category와Folder가data class에서 일반 클래스로 변경되면서 자동 생성되는equals()메서드가 사라졌고, 참조 동등성(reference equality)이 사용됩니다. 결과적으로 새로운Folder인스턴스가 같은 값으로 생성되어도 다른 객체 참조이므로 Compose가 변경을 감지하지 못하고menuText가 갱신되지 않는 문제가 발생합니다.equals()및hashCode()를 명시적으로 구현하거나, 의존성 키 설계를 재검토하여 올바른 동등성 비교가 가능하도록 수정해 주세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/file/src/main/java/com/linku/file/ui/bottom/sheet/_ShareBottomSheet.kt` around lines 177 - 190, The Category and Folder classes are missing equals() and hashCode() implementations which are required for Compose to properly track dependency changes in the remember(selectDepth) call. Implement explicit equals() and hashCode() methods in both the Category class (comparing the category property) and the Folder class (comparing both the category and folder properties) to ensure reference equality is replaced with value equality, allowing Compose to correctly detect when selectDepth changes and update the UI accordingly.
🧹 Nitpick comments (2)
feature/file/src/main/java/com/linku/file/ui/content/LinksGrid.kt (1)
152-178: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
itemsIndexed에 stable key를 지정해 재구성 비용과 아이템 매칭 리스크를 줄여주세요.현재는 인덱스 기반 식별이라 링크 추가/삭제 시 셀 재사용이 흔들릴 수 있습니다.
linkuId기반 key를 지정하는 편이 안전합니다.변경 제안
- itemsIndexed(linkList) { index, link -> + itemsIndexed( + items = linkList, + key = { _, link -> link.linkuId } + ) { index, link -> val itemIndex = index + itemIndexOffsetAs per coding guidelines,
**/*.{kt,kts}: Focus on recomposition cost in Android/Kotlin/Compose code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/file/src/main/java/com/linku/file/ui/content/LinksGrid.kt` around lines 152 - 178, The itemsIndexed lambda in the LinkItemLayout block is relying on index-based identification without providing a stable key parameter, which causes unnecessary recompositions and risks incorrect item reuse when the linkList is modified. Add a key parameter to the itemsIndexed function that uses a stable identifier from the link object (such as link.linkId or link.linkuId) to ensure proper item matching across recompositions and reduce recomposition costs.Source: Coding guidelines
feature/file/src/main/java/com/linku/file/ui/content/FolderGrid.kt (1)
48-63: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
itemsIndexed에 stable key를 추가해 아이템 상태 뒤섞임을 방지해주세요.현재는 인덱스 기반 슬롯 재사용이라 목록 삽입/재정렬 시 셀 내부 상태가 다른 폴더로 이동할 수 있습니다.
folderId같은 안정 키를 지정하는 편이 안전합니다.제안 diff
- itemsIndexed(folderList) { index, folder -> + itemsIndexed( + items = folderList, + key = { _, folder -> folder.folderId } + ) { index, folder -> val itemIndex = index + itemIndexOffsetAs per coding guidelines, "Focus on state management (state hoisting, remember vs rememberSaveable) in Android/Kotlin/Compose code" and "Focus on recomposition cost in Android/Kotlin/Compose code".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/file/src/main/java/com/linku/file/ui/content/FolderGrid.kt` around lines 48 - 63, The itemsIndexed lambda in the FolderGrid composable lacks a stable key parameter, causing item state to shuffle when the folderList is reordered or has items inserted. Add a key parameter to the itemsIndexed call that uses a stable identifier from the folder object (such as folder.folderId or another unique identifier) instead of relying on the default index-based keying. This ensures that compose correctly associates state with the same folder item even when the list is reordered.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@feature/file/src/main/java/com/linku/file/ui/content/LinksGrid.kt`:
- Around line 228-235: The LinksGridTest preview function is directly calling
hiltViewModel() and viewModel() which fails in the preview environment because
there is no Hilt DI container or Android lifecycle available. Refactor the
LinksGrid composable to accept the necessary state and data as parameters
instead of creating ViewModels internally. Then update the LinksGridTest preview
function to remove the hiltViewModel() and viewModel() calls and instead pass
dummy or mock data objects to LinksGrid that satisfy its parameter requirements.
This ensures the preview can render without relying on the DI system.
---
Outside diff comments:
In
`@feature/file/src/main/java/com/linku/file/ui/bottom/sheet/_ShareBottomSheet.kt`:
- Around line 177-190: The Category and Folder classes are missing equals() and
hashCode() implementations which are required for Compose to properly track
dependency changes in the remember(selectDepth) call. Implement explicit
equals() and hashCode() methods in both the Category class (comparing the
category property) and the Folder class (comparing both the category and folder
properties) to ensure reference equality is replaced with value equality,
allowing Compose to correctly detect when selectDepth changes and update the UI
accordingly.
---
Nitpick comments:
In `@feature/file/src/main/java/com/linku/file/ui/content/FolderGrid.kt`:
- Around line 48-63: The itemsIndexed lambda in the FolderGrid composable lacks
a stable key parameter, causing item state to shuffle when the folderList is
reordered or has items inserted. Add a key parameter to the itemsIndexed call
that uses a stable identifier from the folder object (such as folder.folderId or
another unique identifier) instead of relying on the default index-based keying.
This ensures that compose correctly associates state with the same folder item
even when the list is reordered.
In `@feature/file/src/main/java/com/linku/file/ui/content/LinksGrid.kt`:
- Around line 152-178: The itemsIndexed lambda in the LinkItemLayout block is
relying on index-based identification without providing a stable key parameter,
which causes unnecessary recompositions and risks incorrect item reuse when the
linkList is modified. Add a key parameter to the itemsIndexed function that uses
a stable identifier from the link object (such as link.linkId or link.linkuId)
to ensure proper item matching across recompositions and reduce recomposition
costs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ca27cae3-d755-4c8b-aae1-a946248640e6
📒 Files selected for processing (4)
feature/file/src/main/java/com/linku/file/ui/bottom/sheet/_ShareBottomSheet.ktfeature/file/src/main/java/com/linku/file/ui/content/FolderGrid.ktfeature/file/src/main/java/com/linku/file/ui/content/LinksGrid.ktfeature/file/src/main/java/com/linku/file/ui/content/TopFolderGrid.kt
KateteDeveloper
left a comment
KateteDeveloper
left a comment
There was a problem hiding this comment.
_ShareBottomSheet는 일단 완료했습니다~!
미완성인 부분이 있다고 해서 일단 여기까지만 마무리할게요:)
| * @property folder 실제 공유 대상으로 선택된 하위 폴더 정보. | ||
| */ | ||
| private data class Folder(val category: FolderSimpleInfo, val folder: FolderSimpleInfo) : SelectDepth | ||
| private class Folder(override val category: FolderSimpleInfo, val folder: FolderSimpleInfo) : Category(category) { |
There was a problem hiding this comment.
[Q, P1] 여기서 FolderSimpleInfo 응답을 보면
data class FolderSimpleInfo(
val folderId: Long,
val folderName: String,
val parentFolderId: Long,
val isBookmarked: Boolean,
val isSharing: String? = null,
val children: List<FolderSimpleInfo> = emptyList(),
)으로 되어있습니다. 그런데 FolderListResponseDTO는 children 필드가 없습니다!
제가 이해한 게 맞다면 children은 서버 응답값이 아니라 폴더 선택 화면 등에서 트리 구조를 그리기 위해 클라이언트에서 덧붙인 필드로 보입니다.
core 모듈 모델에 있으면 도메인 모델의 "순수성"이 깨진다고 생각합니다:) 그래서 여기서 궁금한게 홰 feature 모듈에서 확장하지 않았는지도 궁금합니다!(질문 입니다!)
feature/file 에서 아예 확장이 필요한 곳에서 아래의 코드로 관리될 수는 없는지 역으로 질문을 드립니다!
data class FolderTreeNode(
val info: FolderSimpleInfo,
val children: List<FolderTreeNode> = emptyList(),
)Category/Folder도 open class 상속 구조로 되어있는데 같은 결로 core 도메인 모델도 향후 open class로 가는 설계는 지양하는 게 어떨지 의견 여쭤봅니다. 도메인 모델이 상속 가능해지면 feature마다 제각각 확장된 서브타입이 생길 수 있고, "이 타입이 진짜 순수 모델인지"를 보장하기 어려워질 것 같습니다.
그리고 Category(open val category: FolderSimpleInfo)에서 category를 굳이 open val로 둔 이유도 궁금합니다. FolderSimpleInfo는 불변 값 객체인데, 실제로 Folder에서도 이 값을 오버라이드해서 다른 값을 주는 게 아니라 생성자에서 받은 값을 그대로 부모에 전달만 하고 있는 것으로 보이는데..! 오버라이드가 실제로 쓰이는 케이스가 있는지, 아니면 단순히 상속 구조를 만들다 보니 같이 열린 건지 궁금합니다.
| private open class Category(open val category: FolderSimpleInfo) : SelectDepth{ | ||
| override val depthOrder = 1 | ||
| } |
There was a problem hiding this comment.
[p3] 참고로 object보다 data object를 쓰면 toString()도 깔끔하게 나온다고 합니다 -클로드-
| private class Folder(override val category: FolderSimpleInfo, val folder: FolderSimpleInfo) : Category(category) { | ||
| override val depthOrder = 2 | ||
| } |
There was a problem hiding this comment.
[Q, P2] 이번 PR 범위는 아닌데 화면 비율 계산 관련해서 질문 드립니다.
private const val FULL_HEIGHT = 917f
private const val SHEET_MAIN_SCREEN_HEIGHT_RATIO = SHEET_MAIN_SCREEN_HEIGHT / FULL_HEIGHT
FULL_HEIGHT가 Figma 디자인 시안의 고정 기준값(412×917)으로 보이는데, 실제 디바이스의
화면 높이를 읽어오는 코드는 안 보입니다. fillMaxHeight(ratio)는 부모 컴포저블의
실측 높이 × ratio로 동작하니, 디바이스마다 곱해지는 베이스 값은 다르지만 비율
자체는 412×917 화면을 기준으로 고정된 값입니다.
이 비율이 다른 종횡비의 화면(더 길쭉한 화면, 더 짧은 화면, 태블릿 등)에도 그대로
곱해지면, 디자인 시안에서 의도했던 여백감/비율감과 달라질 수 있을 것 같습니다.
비율 스케일링이 등비가 아니라 디자인 시안 자체의 가로세로 비율에 종속된 값이라서요.
혹시 의도적으로 이렇게 두신 건지, 아니면 scaler 유틸(Figma 412×917 baseline)을
이 화면에는 적용 안 하신 이유가 있으신지 궁금합니다. scaler를 쓰면 LocalConfiguration이나
BoxWithConstraints로 실제 화면 높이를 받아서 디자인 기준 dp 값을 직접 변환하는
방식이었던 것 같은데, 이 파일에서는 비율(fraction)을 직접 계산해서 fillMaxHeight에
넣는 방식으로 다르게 가셔서 — 두 방식을 통일하는 게 나을지 의견 여쭤봅니다.
물론 scaler도 수정할 점이 있을거라, 아예 이거 수정해서 쓰는게 더 좋지 않을까 싶습니다~!
- 제 생각을 ai에게 정리를 부탁드렸습니다:) -
| private class Folder(override val category: FolderSimpleInfo, val folder: FolderSimpleInfo) : Category(category) { | ||
| override val depthOrder = 2 | ||
| } |
There was a problem hiding this comment.
[p2] 이번 PR은 아닌데 colors: ThemeColorScheme를 매 컴포저블마다 파라미터로 넘기는 패턴 관련해서
의견 드립니다.
val colors = MaterialTheme.linkuColors
이걸 ShareBottomSheetLayout 최상위에서 한 번 받아서, 5~6단계 자식 컴포저블까지
계속 파라미터로 손으로 넘기고 있습니다 (ShareBottomSheetLayout →
ShareBottomSheetMainScreen → LinkShareButton/LinkCopyButton/FolderStructure/
ShareFolderMenu/ShareLink/LinkGenerationButton).
MaterialTheme.linkuColors는 CompositionLocal인데, CompositionLocal의 핵심 가치가
"트리 어디서든 직접 접근 가능해서 prop drilling을 피한다"는 거라서, 지금처럼 매번
파라미터로 넘기면 그 가치를 못 살리고 일반 함수 파라미터처럼 쓰게 되는 것 같습니다.
이렇게 되면:
- 모든 leaf 컴포저블 시그니처에 colors가 박혀서, 함수만 봤을 때 "이게 진짜 필요한
의존성인가"가 헷갈립니다. - Preview 작성 시 colors = MaterialTheme.linkuColors를 매번 또 호출해줘야 합니다.
이미 ThemeProvider 안에 있다면 내부에서 직접 읽으면 될 것 같습니다. - 다른 화면에서 LinkShareButton 같은 컴포넌트를 재사용하려면 호출하는 쪽이 colors를
매번 구해서 넘겨줘야 합니다.
leaf 컴포저블들(LinkShareButton, LinkCopyButton, FolderStructure, ShareFolderMenu,
ShareLink, LinkGenerationButton)에서는 colors 파라미터를 빼고 내부에서 직접
MaterialTheme.linkuColors를 읽는 방향이 어떨지 제안 드립니다.
@composable
private fun LinkShareButton(
modifier: Modifier,
onShare: () -> Unit
) {
val colors = MaterialTheme.linkuColors
// ...
}
다만 테스트에서 다른 색상 스킴을 주입하고 싶거나, 같은 컴포저블이 여러 다른 테마
컨텍스트에서 동시에 쓰일 수 있는 경우라면 파라미터로 넘기는 게 합리적일 수 있는데,
지금 코드에서는 그런 이유가 보이지 않아서 여쭤봅니다. 혹시 의도하신 부분이 있을까요?
| categoryColorMap: Map<String, CategoryColorStyle>, | ||
| onFolderClick: (FolderSimpleInfo) -> Unit, | ||
| FolderLayout: @Composable (folder: FolderSimpleInfo, colorStyle: CategoryColorStyle) -> Unit | ||
| folderLayout: @Composable (folder: FolderSimpleInfo, colorStyle: CategoryColorStyle) -> Unit |
There was a problem hiding this comment.
[P1] 이번 PR에 추가된 내용은 아니지만,
categoryColorMap: Map<String, CategoryColorStyle>호출부에서 매번 새롭게 만드는 코드 아닌지 여쭈어 봅니다! 함수가 호출될 때마다 Map 객체가 메모리에 생성되나 보니 이 방식이 아닌 다른 방식은 어떤지 여쭈어 봅니다! 또 Map 자체가 리컴포지션의 우려가 있다고 생각합니다!(지민이가 이건 나보다 잘 알테니.... 뭐 알아서 잘해주세용 -해줘.-)
| categoryColorMap: Map<String, CategoryColorStyle>, | ||
| onFolderClick: (FolderSimpleInfo) -> Unit, | ||
| FolderLayout: @Composable (folder: FolderSimpleInfo, colorStyle: CategoryColorStyle) -> Unit | ||
| folderLayout: @Composable (folder: FolderSimpleInfo, colorStyle: CategoryColorStyle) -> Unit |
There was a problem hiding this comment.
[Q]
folderLayout: @Composable (folder: FolderSimpleInfo, colorStyle: CategoryColorStyle) -> UnitcolorStyle을 folder와 별개 파라미터로 받고 있는데, 폴더 색상이 사용자가
언제든 변경 가능한 값이라면 isBookmarked처럼 폴더 정보에 포함되어 있는
게 더 안전하지 않을까 싶습니다. 단, colorStyle은 core 도메인 개념이라기보다
UI 표현에 가까우니 core.model.FolderSimpleInfo 자체에 넣기보다는,
feature 모듈에서 FolderSimpleInfo를 합성해서 확장한 모델에 포함시키는
방향이 맞을 것 같습니다.
지금처럼 분리해서 categoryColorMap[folder.folderName]으로 조회하면,
키가 folder.folderName이라 사용자가 폴더 이름을 바꿀 경우 색 매핑이
끊기거나 엉뚱한 색을 가져올 위험이 있을 것 같습니다. feature 쪽 확장
모델에 colorStyle(또는 colorIndex)이 포함되어 내려오면, 이름 기준
조회 자체가 필요 없어지고 folderLayout 시그니처도
folderLayout: @composable (folder: FolderInfoWithColor) -> Unit
처럼 단순해질 것 같습니다. -ai와 함께 한 피드백 입니다.-
There was a problem hiding this comment.
FolderSimpleInfo에 색 정보까지 담겨있는 게 최고지.. 큰 문제 없이 일단 동작이 가능한 상태의 코드니 수정해야해요. 위 코멘트에도 해당하는 내용입니당.
| Row( | ||
| modifier = Modifier | ||
| .fillMaxWidth(), | ||
| horizontalArrangement = if (itemIndex % 2 == 0) Arrangement.Start else Arrangement.End, | ||
| verticalAlignment = Alignment.CenterVertically | ||
| ) { | ||
| folderLayout( | ||
| folder, | ||
| categoryColorMap[folder.folderName] ?: CategoryColorStyle.DEFAULT | ||
| ) | ||
| } |
There was a problem hiding this comment.
[Q] itemsIndexed 안에서 Row + itemIndex % 2로 좌우 정렬을 직접
분기하고 있는데, columns = GridCells.Fixed(2)가 이미 각 아이템을
1열/2열에 배치해주니 이 분기가 중복 로직 아닐까 싶습니다.
좌우 여백 의도(왼쪽 22 / 열 사이 20 / 오른쪽 22)는 LazyVerticalGrid의
contentPadding(horizontal = 22.dp)와 horizontalArrangement =
Arrangement.spacedBy(20.dp)로도 표현 가능할 것 같은데, 이렇게 가면
Row 래핑과 itemIndex % 2 분기를 둘 다 없앨 수 있을 것 같습니다.
추가로, 지금처럼 % 2에 정렬 로직이 묶여 있으면 나중에 열 개수를
바꾸거나(Fixed(3), Adaptive 등) 정렬이 깨질 수 있어서,
Arrangement.spacedBy 방식이 열 개수 변경에도 더 안전할 것 같습니다.
There was a problem hiding this comment.
horizontalArrangement 없이 folderLayout이 전체 공간을 사용하지않고 fillMaxWidth(0.75f) 이런 식으로 일부만 사용하면, 오른쪽 아이템은 중앙쪽으로 치우쳐지는 문제가 생겨서 넣은 거예용
| } | ||
| } | ||
|
|
||
| itemsIndexed(linkList) { index, link -> |
There was a problem hiding this comment.
[P2] FolderGrid에서 짚었던 "Row + % 2 수동 정렬" 패턴이 여기도 똑같이 있어요. GridCells.Fixed(2)가 이미 좌우 칸을 배치해주는데, 또 인덱스 연산이 있습니다~!
| showAddLinkItem: Boolean = !isShareMode, | ||
| isLongClickEnabled: Boolean = !isShareMode, | ||
| itemIndexOffset: Int = if (showAddLinkItem) 1 else 0, |
There was a problem hiding this comment.
[p1] ai가 찾아줘서 피드백 남깁니다:) 저는 발견하지 못한 부분이라...
showAddLinkItem: Boolean = !isShareMode,
itemIndexOffset: Int = if (showAddLinkItem) 1 else 0,item { AddLinkItem(...) }을 그리드 맨 앞에 하나 끼워넣고, 그 뒤에 오는 링크들의 인덱스를 1씩 밀어서(itemIndexOffset) 좌우 정렬을 맞추려는 의도예요. 이게 바로 "Row+%2 정렬"이 왜 깨지기 쉬운지를 보여주는 실제 사례예요 — item을 하나 끼워넣는 순간, 그 뒤 모든 아이템의 짝/홀이 한 칸씩 밀려야 해서 itemIndexOffset이라는 보정값을 수동으로 계산하고 들고 다녀야 합니다.
만약 GridCells.Fixed(2) + Arrangement.spacedBy만 썼다면 이 보정 로직 자체가 필요 없었을 거예요. 그리드가 "지금 몇 번째 칸인지"를 알아서 추적하니까, AddLinkItem을 앞에 끼워넣어도 그 다음 아이템들이 자동으로 올바른 칸에 들어가요. 지금 구조는 이 자동 배치를 손으로 재현하려다가, 끼워넣은 아이템 하나 때문에 오프셋 계산까지 추가로 필요해진 상황이에요. -클로드-
| modifier = Modifier.alpha(1f), | ||
| ) { | ||
| LinkItemLayout( | ||
| link = null |
There was a problem hiding this comment.
[Q, P2] LinkItemLayout이 link: LinkItemInfo? 를 nullable로 받아서,
null일 때(="추가하기" 버튼)와 아닐 때(=실제 링크 카드)를 함수 내부
분기로 같이 처리하고 있는 것 같습니다.
isNotAdder = link != null로 시작해서, 이미지/제목/도메인 텍스트/
clickable 적용 여부/alpha까지 전부 null 체크로 갈라지는데, 사실상
하나의 함수가 두 가지 다른 컴포넌트(실제 링크 카드 / 빈 추가 버튼)
역할을 겸하고 있는 것 같습니다.
게다가 LinksGrid.kt의 AddLinkItem에서는 LinkItemLayout(link = null)을
부른 다음 그 위에 또 Image/Text를 padding(top = 103.dp),
padding(top = 147.dp)로 겹쳐 그리고 있어서, "빈 상태를 표현하는 책임"이
LinkItemLayout 내부와 호출부 양쪽에 나뉘어 있는 것 같습니다.
매직넘버로 위치를 맞추고 있어서 LinkItemLayout의 내부 레이아웃이
바뀌면 같이 깨질 수 있는 결합도도 있어 보입니다.
LinkItemLayout은 link: LinkItemInfo(non-null)만 받는 순수 카드
컴포넌트로 두고, "추가하기" 버튼은 AddLinkItem이 자체적으로
완전히 구현(카드와 동일한 크기/모양만 Modifier로 맞춤)하는 방향이
어떨지 의견 여쭤봅니다. -왜 이게 null인지 궁금해하다가 클로드와 찾은 오류 입니다:)-
There was a problem hiding this comment.
[P2] 이전에 도메인 이미지 캐싱 기억나시나요?
이 PR 보다가 LinkGrid에서
val domainIcon = link?.let{ domainLogoPainterOrNull(it.url) }이걸 사용하고 있는 것을 발견했습니다. 일단 캐싱 자체 문제는 나중에 논의하고 link?.let이 remember로 안 감싸지면 매번 리컴포지션에서 다시 계산됩니다!!
val domainIcon = remember(link?.url) { link?.let { domainLogoPainterOrNull(it.url) } }으로 수정을 제안을 드립니다:)
아 그리고
@Composable
fun LinkItemTag(tag: String)이게 LinkItemLayout 컴포저블 함수 안에 로컬 함수로 또 구현되어 있습니다.
차라리 따로 분리해서 구현하는 게 좋을 것 같습니다!
| categorizationModalWindowVisible = true | ||
| } | ||
| }, | ||
| onLinkClick = { link -> |
There was a problem hiding this comment.
[Q] 여기서 클릭 콜벡 자체를 파일 뷰모델에서 있어야 하는 이유가 궁금합니다!
아 FileViewModel 찾아보니까, 이런 코드가 있네요
var onLinkClick: ((Long) -> Unit)? = null
fun registeronLinkClick(callback: (Long) -> Unit) {
onLinkClick = callback
}콜백 자체를 뷰모델이 들고 다닐 이유가 있어야 할 이유가 있을까요? 단순 클릭 이벤트라면 클릭 발생 사실만 들고 다니면 될 것 같은데..(Channel이나 sharedFlow) 실제 클릭 이벤트는 여기서 가지고 있는게 안전할 것 같다는 생각이 듭니다!
이 밑은 ai가 짚어줬습니다:)
이 콜백이 등록되는 쪽(registeronLinkClick을 호출하는 곳)에서 navController 같은
UI/Activity 컨텍스트를 캡처하고 있다면, ViewModel이 더 긴 생명주기를 가지는데도
더 짧은 생명주기의 객체를 참조하는 람다를 들고 있게 되어 메모리 누수 위험이 있을 것
같습니다. 또한 콜백을 var 하나로만 들고 있어서, 화면이 재진입되며
registeronLinkClick이 다시 호출되면 이전 등록을 덮어쓰게 될 텐데, 이게 의도된
동작인지도 궁금합니다.
There was a problem hiding this comment.
아 그리고 registeronLinkClick 여기 오타 있어용
registerOnLinkClick으로 수정해주세요(해줘해줘해줘)
| ) { | ||
| val colors = MaterialTheme.linkuColors | ||
|
|
||
| Row( |
There was a problem hiding this comment.
[P3] AddLinkItem 안에 Row > Box > Box > Box로 4단 중첩이 있는데,
까보니 대부분 불필요한 래퍼로 보입니다.
- 가장 바깥 Row: 자식이 Box 하나뿐이라 가로 배치 기능을 전혀 안 씁니다
- contentAlignment = TopCenter인 Box: 자식들이 각자 padding(top = ...)
으로 위치를 직접 지정하고 있어서 이 정렬값이 적용될 일이 없습니다 - alpha(1f)인 Box: 1f는 완전 불투명, 즉 아무 효과가 없는 값입니다
(LinkItemTest 프리뷰의 주석 처리된 alpha(0.35f) 흔적을 보면
되돌리다가 Box만 남은 것 같습니다)
noRippleClickable과 contentAlignment를 가진 Box 하나로 합치면
4단 중첩이 1단으로 줄고 동일한 결과가 나올 것 같습니다.
| if (editStateViewModel.isEditMode) { | ||
| folderStateViewModel.updateReadyToUpdateTopFolder(folder) | ||
| folderStateViewModel.updateTopFolderEditBottomSheetVisible(true) | ||
| } else { | ||
| fileViewModel.getFoldersAndNotCategorizationLinks(folder.folderId) | ||
| folderStateViewModel.updateSelectedTopFolder(folder) | ||
| folderStateViewModel.updateFolderState(FolderState.BOTTOM) | ||
| } |
There was a problem hiding this comment.
[Q, P2] TopFolderGrid의 클릭 핸들러에서
if (editStateViewModel.isEditMode) {
folderStateViewModel.updateReadyToUpdateTopFolder(folder)
folderStateViewModel.updateTopFolderEditBottomSheetVisible(true)
} else {
fileViewModel.getFoldersAndNotCategorizationLinks(folder.folderId)
folderStateViewModel.updateSelectedTopFolder(folder)
folderStateViewModel.updateFolderState(FolderState.BOTTOM)
}
이 분기(편집 모드인지 확인해서 행동을 결정하는 판단)가 Composable
안에 있는데, MVVM에서는 이런 판단도 ViewModel의 책임이라고 생각해서
의견 여쭤봅니다.
지금 FolderStateViewModel은 updateX(newValue) 형태의 단순 setter만
노출하고 있어서, "어떤 상태들을 어떤 순서로 바꿀지 결정하는 로직"을
View가 대신 짊어지고 있는 것 같습니다. ViewModel에
onTopFolderClick(folder: FolderSimpleInfo) 같은 단일 진입점을
만들고, ViewModel이 이미 들고 있는 isEditMode를 내부에서 참조해서
분기/상태 변경 순서를 처리하면, Composable은 클릭 이벤트만
전달하면 될 것 같습니다.
추신 : mvi 구조였으면 "TopFolderClicked(folder)" 인텐트 하나로
끝났을 텐데... 안타깝....
- `VerticalGrid`와 `Column` 기반의 수동 레이아웃을 `LazyVerticalGrid`로 전환하여 스크롤 성능 및 구조 개선 - `AddBottomFolderItem`, `BottomFolderItem` 컴포넌트 분리를 통한 가독성 및 재사용성 향상 - `GridItemSpan`을 사용하여 "분류되지 않은 링크" 섹션 헤더가 전체 너비를 차지하도록 수정 - `folderIndexById`를 `remember`로 관리하여 폴더 삭제 시 인덱스 조회 로직 최적화 - 레이아웃에 사용되는 패딩 및 간격 수치를 상수로 정의하고 `contentPadding` 적용 - 기존의 수동적인 좌우 정렬(`i % 2 == 0`) 방식을 그리드 시스템 표준 방식으로 변경
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
feature/file/src/main/java/com/linku/file/ui/content/BottomFolderGrid.kt (1)
281-281: 📐 Maintainability & Code Quality | 🔵 Trivial
hiltViewModelimport 최신화 요청
BottomFolderGrid.kt파일의hiltViewModel()사용을androidx.hilt.navigation.compose.hiltViewModel대신 최신androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel으로 변경하세요.- import androidx.hilt.navigation.compose.hiltViewModel + import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/file/src/main/java/com/linku/file/ui/content/BottomFolderGrid.kt` at line 281, The BottomFolderGrid composable is still using the old hiltViewModel import path; update the import used by BottomFolderGrid and its Hilt-backed view model hookup to androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel instead of androidx.hilt.navigation.compose.hiltViewModel so the existing hiltViewModel() call resolves to the newer API.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@feature/file/src/main/java/com/linku/file/ui/content/BottomFolderGrid.kt`:
- Around line 86-90: `FolderGrid` 안의 `itemsIndexed` 가 인덱스를 키로 사용해
`BottomFolderItem` 의 remember 상태가 항목 이동/삭제 시 잘못 재사용됩니다. `FolderGrid`(및 이를 호출하는
`BottomFolderGrid`)에서 각 폴더의 고유 식별자(예: folder id)를 `key` 인자로 넘기도록 수정해, 위치가 아니라 동일
항목 기준으로 상태가 유지되게 하세요.
- Around line 110-112: The subfolder delete flow is still using the passed index
to update local state, which can remove the wrong item if the list changes after
the server call. Update the `onDelete` path in `BottomFolderGrid` and the
`FileViewModel.deleteSubfolder(folderId, index)` API to identify the deleted
item by `folderId` instead of index, and make the `_subFolders` refresh/filter
logic in `FileViewModel` remove matching IDs rather than relying on positional
deletion.
---
Nitpick comments:
In `@feature/file/src/main/java/com/linku/file/ui/content/BottomFolderGrid.kt`:
- Line 281: The BottomFolderGrid composable is still using the old hiltViewModel
import path; update the import used by BottomFolderGrid and its Hilt-backed view
model hookup to androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel instead
of androidx.hilt.navigation.compose.hiltViewModel so the existing
hiltViewModel() call resolves to the newer API.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 03119834-cbd3-4348-b72f-ee7d1d98c0b4
📒 Files selected for processing (1)
feature/file/src/main/java/com/linku/file/ui/content/BottomFolderGrid.kt
- `FolderIcons.kt` 파일을 `ui/content` 패키지에서 `ui/item` 패키지로 이동 - 파일 이동에 따른 패키지 경로 선언 수정 및 `Modifier` 참조 방식 등 일부 코드 정돈
- `TopFolderItemLayout`을 `CategoryItemLayout`으로 변경하여 용도 명확화 - `BottomFolderItemLayout`을 `MyFolderItemLayout`으로 변경하여 용도 명확화 - `FolderItemLayout.kt` 내에서 더 이상 사용하지 않는 아이콘 관련 임포트 제거 - 테스트 및 프리뷰 코드 내 변경된 컴포넌트 명칭 반영
- `FolderGrid` 최상위 컴포넌트를 삭제하고 `LazyGridScope`의 확장 함수 형태로 통합 - `categoryColorMap` 매개변수를 삭제하여 UI 컴포넌트 내의 색상 매핑 의존성 제거 - `folderLayout` 슬롯 API에서 `CategoryColorStyle` 매개변수 제거 - 하드코딩된 패딩 상수(`MIDDLE_PADDING`) 및 `PaddingValues` 설정 삭제 - `inline` 키워드 추가 및 `crossinline` 람다 적용을 통한 성능 최적화 - `Row` 내부의 `verticalAlignment` 속성 제거 및 레이아웃 배치 로직 간소화
- `TopFolderGrid.kt` 파일을 삭제하고 `CategoryGrid.kt`로 대체하여 UI 구조 재설계 - `TopFolderItemLayout` 컴포넌트를 `CategoryItemLayout`으로 명칭 변경 및 관련 참조 수정 - `CategoryGrid` 내부에 `LazyVerticalGrid`를 직접 구현하여 그리드 레이아웃 구성 방식 최적화 - `collectAsStateWithLifecycle`을 적용하여 Compose 상태 수집 시 생명주기 안정성 강화 - `ShareBottomSheet`에서 기존 TopFolder 관련 UI 요소를 `CategoryItemLayout`으로 교체 - `CategoryColorStyle.DEFAULT` 상수를 사용하여 기본 컬러 스타일 지정 로직 개선
- `LinksGrid`를 `ClassifiedLinksGrid`로 명칭 변경 및 관련 파일명 수정 - `LazyGridScope` 내에서 공통으로 사용할 수 있는 `LinkGrid` 확장 함수를 별도 파일로 분리 - `LinkItemLayout`에 `BoxWithConstraints`를 도입하여 부모 크기에 따라 가변적으로 조절되는 반응형 스케일링 로직 구현 - `LinkItemLayout` 내 하드코딩된 크기 값을 비율(aspectRatio) 및 상대 수치로 계산하도록 개선 - `ClassifiedLinksGrid`에서 `by` 위임 프로퍼티를 사용하여 상태 수집 방식 개선 - `AddLinkItem` 컴포넌트의 레이아웃 구조 단순화 및 비율 기반 크기 조정 적용
- `BottomFolderGrid.kt` 파일 및 컴포넌트 이름을 `MyFoldersGrid`로 변경 - `BottomFolderItem`을 `MyFolderItem`으로, `BottomFolderItemLayout`을 `MyFolderItemLayout`으로 각각 명칭 변경 - `MyFoldersGrid`에서 `onFolderAdd` 콜백을 제거하고 `folderStateViewModel`을 통해 직접 상태를 업데이트하도록 변경 - `AddBottomFolderItem` 컴포넌트의 클릭 리스너와 편집 모드 로직을 호출부의 `Modifier` 주입 방식으로 개선 - `FolderGrid` 및 `LinkGrid`를 사용하여 그리드 내의 폴더와 링크 아이템 표시 로직을 단순화 - `MyFolderItem`에서 폴더 삭제 시 인덱스 기반이 아닌 `folderId`만 전달하도록 수정 - `ShareBottomSheet` 및 `FileScreen` 등 연관된 파일의 컴포넌트 참조 이름 수정 - 미리보기(`Preview`) 함수에서 `hiltViewModel()` 대신 `viewModel()`을 사용하도록 변경
- `BoxWithConstraints`를 사용하여 가용 너비에 기반한 `horizontalSpacing` 동적 계산 로직 추가 - `ITEM_RATIO` 상수를 정의하여 다양한 화면 크기에서 일관된 아이템 비율 유지 - `ClassifiedLinksGrid`, `CategoryGrid`, `MyFoldersGrid` 컴포넌트의 그리드 레이아웃 구조 개선 - `MyFoldersGrid`에 `modifier` 및 `contentPadding` 파라미터를 추가하여 외부 주입이 가능하도록 변경 - `CategoryGrid` 내 아이템의 불필요한 고정 비율 `modifier`를 제거하고 `fillMaxSize()`로 수정
- `FolderGrid.kt` 및 `LinkGrid.kt` 파일 삭제 - `ClassifiedLinksGrid`, `CategoryGrid`, `MyFoldersGrid`에서 커스텀 래퍼 대신 `LazyGridScope.items`를 직접 사용하도록 수정 - `CategoryGrid` 내 `LocalLayoutDirection` 참조 및 패딩 계산 로직 구조 개선 (with 구문 사용) - 불필요한 인덱스 계산(`itemIndexOffset`) 및 내부 `Row` 배치를 제거하고 Compose 표준 Grid 시스템 활용하도록 변경
- `ClassifiedLinksGrid`, `CategoryGrid`, `MyFoldersGrid` 등 주요 그리드 컴포넌트에 대한 역할 및 매개변수 KDoc 작성 - `FolderItemLayout`, `LinkItemLayout` 등 개별 아이템 레이아웃 컴포넌트와 내부 서브 컴포저블에 대한 상세 설명 추가 - 그리드 내 너비 계산 로직, 편집 모드 분기 처리 등 복잡한 UI 로직에 대한 인라인 주석 보강 - 각 파일 내 주요 상수(패딩, 비율 등) 및 Preview 함수에 대한 설명 추가 - 파일 단위의 역할을 명시하는 파일 상단 주석 추가
- `CategoryGrid`, `MyFoldersGrid`, `ClassifiedLinksGrid`에서 ViewModel 직접 참조를 제거하고 명시적인 데이터와 콜백을 받도록 변경 - `FileScreen`에서 상태 수집(`collectAsStateWithLifecycle`) 및 ViewModel 로직을 수행하도록 역할 위임 - 각 그리드 컴포넌트에 폴더/링크 클릭, 삭제, 편집 등의 이벤트 처리를 위한 람다 매개변수 추가 - UI 로직과 비즈니스 로직을 분리하여 컴포넌트의 재사용성 및 테스트 가능성 향상 - 의존성 변경에 따른 각 컴포넌트의 Preview(Test) 코드 수정
- `ClassifiedLinksGrid`, `MyFoldersGrid` 등 그리드 컴포넌트 내 레이아웃 계산 및 상태 관리 로직 설명 추가 - `FolderItemLayout`, `LinkItemLayout` 내 디자인 기준 기반 반응형 스케일링(scaling) 공식 상세 기술 - 폴더 레이어 중첩 방식 및 마스크 이미지 색상 적용(BlendMode) 로직 주석 보완 - 링크 카드 내 썸네일 로딩 처리 및 도메인 아이콘 매핑 구조 설명 작성 - 각 UI 컴포넌트의 클릭 이벤트 위임 및 모달 창 동작 흐름 명시 - 프리뷰(Preview) 컴포저블의 역할 및 데이터 구성 방식 주석 추가
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/file/src/main/java/com/linku/file/ui/item/FolderItemLayout.kt (1)
146-153: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win레이어 padding 계산이
LayoutDirection.Ltr로 고정되어 있습니다
calculateStartPadding(LayoutDirection.Ltr)/calculateEndPadding(LayoutDirection.Ltr)가 현재 레이아웃 방향을 무시하고 항상 LTR을 기준으로 계산합니다. RTL 로케일에서는 폴더 레이어의 좌우 오프셋이 의도와 다르게 표시될 수 있습니다.LocalLayoutDirection.current를 사용하는 것이 안전합니다.🌐 제안
+ val layoutDirection = LocalLayoutDirection.current Surface( modifier = Modifier .padding( PaddingValues( - start = s(padding.calculateStartPadding(LayoutDirection.Ltr)), + start = s(padding.calculateStartPadding(layoutDirection)), top = s(padding.calculateTopPadding()), - end = s(padding.calculateEndPadding(LayoutDirection.Ltr)), + end = s(padding.calculateEndPadding(layoutDirection)), bottom = s(padding.calculateBottomPadding())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/file/src/main/java/com/linku/file/ui/item/FolderItemLayout.kt` around lines 146 - 153, The padding calculation in FolderItemLayout currently hardcodes LayoutDirection.Ltr, so the layer offsets ignore the active layout direction. Update the padding block in FolderItemLayout to use the current layout direction from LocalLayoutDirection.current when calling calculateStartPadding and calculateEndPadding, so RTL and LTR render correctly.
🧹 Nitpick comments (7)
feature/file/src/main/java/com/linku/file/ui/content/CategoryGrid.kt (2)
25-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win간격 계산 로직(
INTER_LAYER_PADDING,ITEM_RATIO,BoxWithConstraints기반 spacing 계산)이ClassifiedLinksGrid.kt,MyFoldersGrid.kt에도 거의 동일하게 중복됩니다.세 파일 모두 동일한 상수값과 동일한
horizontalPadding/availableWidth/horizontalSpacing계산 로직을 갖고 있습니다. 공통 유틸(예:rememberGridHorizontalSpacing(contentPadding, ratio)같은 헬퍼 컴포저블/함수)로 추출하면 향후 비율/여백 값을 한 곳에서만 관리할 수 있습니다.Also applies to: 30-30, 68-81
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/file/src/main/java/com/linku/file/ui/content/CategoryGrid.kt` at line 25, The grid spacing logic in CategoryGrid, ClassifiedLinksGrid, and MyFoldersGrid is duplicated, including INTER_LAYER_PADDING, ITEM_RATIO, and the BoxWithConstraints-based horizontalPadding/availableWidth/horizontalSpacing calculation. Extract this into a shared helper such as rememberGridHorizontalSpacing(contentPadding, ratio) or a small utility function, then update the three grid composables to call it so the spacing and ratio values are maintained in one place.
92-114: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
items(categories)에key파라미터가 없어 재정렬 시 불필요한 recomposition 위험.
FileViewModel.parentFolders는isBookmarked기준 내림차순으로 정렬되므로, 사용자가 북마크를 토글하면 리스트 순서가 바뀝니다.key가 없으면 Compose가 위치 기반으로 아이템을 재사용해 애니메이션/상태가 잘못 매핑될 수 있습니다.folder.folderId를 key로 지정하는 것을 권장합니다.♻️ 제안하는 수정
- items(categories) { folder -> + items(categories, key = { it.folderId }) { folder ->As per coding guidelines, "Focus on unnecessary recomposition and performance issues in Android/Kotlin/Compose code".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/file/src/main/java/com/linku/file/ui/content/CategoryGrid.kt` around lines 92 - 114, The CategoryGrid composable’s items(categories) block is missing a stable key, so reordering when bookmarks change can cause incorrect item reuse and extra recomposition. Update the items call in CategoryGrid to provide a unique key based on folder.folderId so each CategoryItemLayout is tracked by identity instead of position. This should be applied where folder is rendered in the grid, alongside the existing noRippleClickable/onBookmarkClick handling.Source: Coding guidelines
feature/file/src/main/java/com/linku/file/ui/item/LinkItemLayout.kt (3)
125-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win클릭 콜백에 남은 디버그 로그
Log.d("LinkItemLayout", ...)가 클릭 콜백 안에 그대로 남아 있습니다. 디버그용 아티팩트이므로 제거하거나 디버그 빌드 전용 로깅으로 감싸는 것을 권장합니다.🧹 제안
onClick = { link.linkuId.let { - Log.d("LinkItemLayout", "아이템 클릭: \"savelinkresult/${it}\"") onClick(link) } },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/file/src/main/java/com/linku/file/ui/item/LinkItemLayout.kt` around lines 125 - 129, Remove the leftover debug logging from the click handler in LinkItemLayout’s onClick block, or guard it so it only runs in debug builds. Update the click callback around link.linkuId.let to avoid calling Log.d("LinkItemLayout", ...) in production while preserving the existing onClick(link) behavior.
104-110: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
ImageRequest를 매 리컴포지션마다 재생성
painter(ImageRequest)가remember없이 매 리컴포지션마다 새로 빌드됩니다.link가 바뀌지 않는 한 동일한 요청이므로link?.linkuImageUrl(또는link)를 키로remember에 감싸는 것을 권장합니다.As per coding guidelines, "Focus on unnecessary recomposition and performance issues in Android/Kotlin/Compose code".
♻️ 제안
- val painter = ImageRequest.Builder(LocalContext.current) - .data(link?.linkuImageUrl) // url이 null일 수도 있음 - .crossfade(true) - .placeholder(R.drawable.link_categorization_default) - .error(R.drawable.link_categorization_default) - .fallback(R.drawable.link_categorization_default) // null이면 이거 표시 - .build() + val context = LocalContext.current + val painter = remember(link?.linkuImageUrl) { + ImageRequest.Builder(context) + .data(link?.linkuImageUrl) // url이 null일 수도 있음 + .crossfade(true) + .placeholder(R.drawable.link_categorization_default) + .error(R.drawable.link_categorization_default) + .fallback(R.drawable.link_categorization_default) // null이면 이거 표시 + .build() + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/file/src/main/java/com/linku/file/ui/item/LinkItemLayout.kt` around lines 104 - 110, `LinkItemLayout`의 `ImageRequest`가 매 리컴포지션마다 새로 생성되고 있습니다. `painter` 생성 부분을 `remember`로 감싸 `LocalContext.current`와 `link?.linkuImageUrl`(또는 `link`)를 키로 사용해, 이미지 요청이 실제로 바뀔 때만 다시 빌드되도록 수정하세요. `ImageRequest.Builder`와 `painter` 선언 위치를 기준으로 캐시되게 정리하면 됩니다.Source: Coding guidelines
176-204: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
LinkItemTag도 로컬 컴포저블 정의로,LazyRow내부 반복 호출 시 리컴포지션 최적화를 방해합니다
FolderItemLayout.kt의FolderLayerBox와 동일한 패턴으로,LinkItemTag가LinkItemLayout본문 내부에 로컬@Composable함수로 정의되어items(tags)안에서 호출됩니다. 태그 칩 개수가 늘어날수록 매 리컴포지션마다 함수가 새로 정의되어 스킵 최적화 혜택을 받지 못합니다. 최상위(또는 private) 컴포저블로 분리하고scale을 파라미터로 전달하는 것을 권장합니다.As per coding guidelines, "Focus on unnecessary recomposition and performance issues in Android/Kotlin/Compose code".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/file/src/main/java/com/linku/file/ui/item/LinkItemLayout.kt` around lines 176 - 204, `LinkItemTag` is defined as a local `@Composable` inside `LinkItemLayout`, so it gets recreated during `LazyRow` item recompositions and prevents Compose skip optimizations. Move `LinkItemTag` out to a top-level or private composable (similar to the `FolderLayerBox` refactor pattern), keep it reusable from `LinkItemLayout`, and pass any needed sizing/scaling values such as `scale` as parameters instead of capturing them from the parent scope.Source: Coding guidelines
feature/file/src/main/java/com/linku/file/ui/item/FolderItemLayout.kt (2)
113-114: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win미사용 변수
tmp제거 권장
tmp = this는 "스코프 사용 경고를 피하기 위한 값"이라는 주석이 붙어 있지만 이후 어디서도 참조되지 않아 실제로 아무 역할도 하지 않습니다. 향후 유지보수 시 혼란을 줄 수 있습니다.♻️ 제안
BoxWithConstraints(Modifier.fillMaxSize()) { - - // BoxWithConstraintsScope 참조를 보관해 스코프 사용 경고를 피하기 위한 값입니다. - val tmp = this - // 현재 실제 너비/높이에 맞춰 디자인 기준 크기 대비 스케일을 계산합니다. val scaleW = maxWidth / baseW🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/file/src/main/java/com/linku/file/ui/item/FolderItemLayout.kt` around lines 113 - 114, The local temporary value assigned in FolderItemLayout is unused and should be removed. Update the BoxWithConstraintsScope-related code in FolderItemLayout so it no longer declares tmp = this, and if needed, keep the scope usage warning suppression by restructuring the surrounding code instead of storing an unused reference.
134-169: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
FolderLayerBox를 로컬 컴포저블로 정의하는 방식은 리컴포지션 스킵을 방해합니다
FolderLayerBox가FolderItemLayout본문(그것도BoxWithConstraints내부) 안에서 로컬@Composable fun으로 선언되어 있습니다. 그리드에서 폴더 카드가 다수 렌더링될 때 이 함수는 매 리컴포지션마다 새로 정의되어, Compose 컴파일러가 제공하는 안정적인 스킵/메모이제이션 최적화를 받을 수 없습니다. 최상위(또는 private) 컴포저블로 분리하고scale을 파라미터로 전달하는 것을 권장합니다.As per coding guidelines, "Focus on unnecessary recomposition and performance issues in Android/Kotlin/Compose code" and "Focus on recomposition cost in Android/Kotlin/Compose code".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/file/src/main/java/com/linku/file/ui/item/FolderItemLayout.kt` around lines 134 - 169, `FolderLayerBox` is declared as a local composable inside `FolderItemLayout`, which causes it to be recreated on every recomposition and reduces Compose skip/memoization opportunities. Move `FolderLayerBox` out to a top-level or private composable so it has a stable identity, and pass any needed values such as the current scale/constraints as parameters instead of capturing them from `BoxWithConstraints`. Keep the rendering logic and styling the same, but make `FolderItemLayout` call the extracted `FolderLayerBox` composable directly.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@feature/file/src/main/java/com/linku/file/FileScreen.kt`:
- Around line 88-90: Default category color handling is inconsistent in
FileScreen; it falls back to CategoryColorStyle.categoryStyleList[0] while other
screens use CategoryColorStyle.DEFAULT. Update the selectedTopFolderColorStyle
fallback in FileScreen.kt to use the same default source as the other screens,
so the color behavior is consistent across the app.
- Around line 166-178: The paging cursor used by FileViewModel for subfolders is
being shared across different folder contexts, causing
`getFoldersAndNotCategorizationLinks()` and `getLinks()` to interfere with each
other. Update the `FileScreen` flow where `onFolderClick` switches folders and
the related `FileViewModel` methods so `_subFoldersCursor` is reset to null or
maintained per `folderId` whenever the selected folder changes. Ensure
`getLinks()`, `getFoldersAndNotCategorizationLinks()`, and any cursor update
logic in `FileViewModel` no longer reuse the same cursor across different
folders.
- Around line 83-90: The selected top folder color lookup in FileScreen is
indexing categoryColorMap with a nullable folder name, which should be handled
safely. Update the selectedTopFolderColorStyle computation near the
collectAsStateWithLifecycle state reads to first unwrap
folderStateViewModel.selectedTopFolder?.folderName with let, then look up the
color in categoryColorMap, and fall back to
CategoryColorStyle.categoryStyleList[0] when the folder name is null or missing.
In
`@feature/file/src/main/java/com/linku/file/ui/bottom/sheet/ShareBottomSheet.kt`:
- Around line 278-283: In ShareBottomSheet’s CategoryItemLayout preview card,
the bookmark icon is shown even though its onBookmark handler is a no-op, which
makes the UI look interactive but do nothing; update this call site to hide the
bookmark area for this share preview by setting visibleBookmarked to false, and
keep the existing folder/category setup intact so the card remains a read-only
preview.
In `@feature/file/src/main/java/com/linku/file/ui/content/MyFoldersGrid.kt`:
- Around line 162-184: `items(folders)` is missing a stable key, so Compose may
reuse the wrong slot state and carry `MyFolderItem`’s local
`deleteModalWindowVisible` across folder changes. Update the `items` call in
`MyFoldersGrid` to use a stable identifier such as `folder.folderId` as the key
so each folder keeps its own composition state. This will prevent the delete
confirmation modal inside `MyFolderItem` from appearing on the wrong folder
after deletions or reordering.
---
Outside diff comments:
In `@feature/file/src/main/java/com/linku/file/ui/item/FolderItemLayout.kt`:
- Around line 146-153: The padding calculation in FolderItemLayout currently
hardcodes LayoutDirection.Ltr, so the layer offsets ignore the active layout
direction. Update the padding block in FolderItemLayout to use the current
layout direction from LocalLayoutDirection.current when calling
calculateStartPadding and calculateEndPadding, so RTL and LTR render correctly.
---
Nitpick comments:
In `@feature/file/src/main/java/com/linku/file/ui/content/CategoryGrid.kt`:
- Line 25: The grid spacing logic in CategoryGrid, ClassifiedLinksGrid, and
MyFoldersGrid is duplicated, including INTER_LAYER_PADDING, ITEM_RATIO, and the
BoxWithConstraints-based horizontalPadding/availableWidth/horizontalSpacing
calculation. Extract this into a shared helper such as
rememberGridHorizontalSpacing(contentPadding, ratio) or a small utility
function, then update the three grid composables to call it so the spacing and
ratio values are maintained in one place.
- Around line 92-114: The CategoryGrid composable’s items(categories) block is
missing a stable key, so reordering when bookmarks change can cause incorrect
item reuse and extra recomposition. Update the items call in CategoryGrid to
provide a unique key based on folder.folderId so each CategoryItemLayout is
tracked by identity instead of position. This should be applied where folder is
rendered in the grid, alongside the existing noRippleClickable/onBookmarkClick
handling.
In `@feature/file/src/main/java/com/linku/file/ui/item/FolderItemLayout.kt`:
- Around line 113-114: The local temporary value assigned in FolderItemLayout is
unused and should be removed. Update the BoxWithConstraintsScope-related code in
FolderItemLayout so it no longer declares tmp = this, and if needed, keep the
scope usage warning suppression by restructuring the surrounding code instead of
storing an unused reference.
- Around line 134-169: `FolderLayerBox` is declared as a local composable inside
`FolderItemLayout`, which causes it to be recreated on every recomposition and
reduces Compose skip/memoization opportunities. Move `FolderLayerBox` out to a
top-level or private composable so it has a stable identity, and pass any needed
values such as the current scale/constraints as parameters instead of capturing
them from `BoxWithConstraints`. Keep the rendering logic and styling the same,
but make `FolderItemLayout` call the extracted `FolderLayerBox` composable
directly.
In `@feature/file/src/main/java/com/linku/file/ui/item/LinkItemLayout.kt`:
- Around line 125-129: Remove the leftover debug logging from the click handler
in LinkItemLayout’s onClick block, or guard it so it only runs in debug builds.
Update the click callback around link.linkuId.let to avoid calling
Log.d("LinkItemLayout", ...) in production while preserving the existing
onClick(link) behavior.
- Around line 104-110: `LinkItemLayout`의 `ImageRequest`가 매 리컴포지션마다 새로 생성되고 있습니다.
`painter` 생성 부분을 `remember`로 감싸 `LocalContext.current`와 `link?.linkuImageUrl`(또는
`link`)를 키로 사용해, 이미지 요청이 실제로 바뀔 때만 다시 빌드되도록 수정하세요. `ImageRequest.Builder`와
`painter` 선언 위치를 기준으로 캐시되게 정리하면 됩니다.
- Around line 176-204: `LinkItemTag` is defined as a local `@Composable` inside
`LinkItemLayout`, so it gets recreated during `LazyRow` item recompositions and
prevents Compose skip optimizations. Move `LinkItemTag` out to a top-level or
private composable (similar to the `FolderLayerBox` refactor pattern), keep it
reusable from `LinkItemLayout`, and pass any needed sizing/scaling values such
as `scale` as parameters instead of capturing them from the parent scope.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 32d3249d-713d-4ac1-9d08-76b7b2736be5
📒 Files selected for processing (13)
feature/file/src/main/java/com/linku/file/FileScreen.ktfeature/file/src/main/java/com/linku/file/FileViewModel.ktfeature/file/src/main/java/com/linku/file/ui/bottom/sheet/ShareBottomSheet.ktfeature/file/src/main/java/com/linku/file/ui/content/BottomFolderGrid.ktfeature/file/src/main/java/com/linku/file/ui/content/CategoryGrid.ktfeature/file/src/main/java/com/linku/file/ui/content/ClassifiedLinksGrid.ktfeature/file/src/main/java/com/linku/file/ui/content/FolderGrid.ktfeature/file/src/main/java/com/linku/file/ui/content/LinksGrid.ktfeature/file/src/main/java/com/linku/file/ui/content/MyFoldersGrid.ktfeature/file/src/main/java/com/linku/file/ui/content/TopFolderGrid.ktfeature/file/src/main/java/com/linku/file/ui/item/FolderIcons.ktfeature/file/src/main/java/com/linku/file/ui/item/FolderItemLayout.ktfeature/file/src/main/java/com/linku/file/ui/item/LinkItemLayout.kt
💤 Files with no reviewable changes (4)
- feature/file/src/main/java/com/linku/file/ui/content/TopFolderGrid.kt
- feature/file/src/main/java/com/linku/file/ui/content/BottomFolderGrid.kt
- feature/file/src/main/java/com/linku/file/ui/content/LinksGrid.kt
- feature/file/src/main/java/com/linku/file/ui/content/FolderGrid.kt
| val categoryColorMap by fileViewModel.categoryColorMap.collectAsStateWithLifecycle() | ||
| val parentFolders by fileViewModel.parentFolders.collectAsStateWithLifecycle() | ||
| val subFolders by fileViewModel.subFolders.collectAsStateWithLifecycle() | ||
| val links by fileViewModel.links.collectAsStateWithLifecycle() | ||
| val notCategorizationLinks by fileViewModel.notCategorizationLinks.collectAsStateWithLifecycle() | ||
| val selectedTopFolderColorStyle = | ||
| categoryColorMap[folderStateViewModel.selectedTopFolder?.folderName] | ||
| ?: CategoryColorStyle.categoryStyleList[0] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the referenced file and related symbols/types.
git ls-files 'feature/file/src/main/java/com/linku/file/FileScreen.kt' 'feature/file/src/main/java/com/linku/file/ShareBottomSheet.kt' '**/*FileViewModel*' '**/*FolderStateViewModel*' '**/*CategoryColorStyle*' | sed 's#^\./##'
echo
echo '--- FileScreen.kt (around the referenced lines) ---'
sed -n '70,100p' feature/file/src/main/java/com/linku/file/FileScreen.kt
echo
echo '--- ShareBottomSheet.kt around the referenced pattern ---'
sed -n '250,280p' feature/file/src/main/java/com/linku/file/ShareBottomSheet.kt
echo
echo '--- Search for categoryColorMap declaration/usages ---'
rg -n "categoryColorMap|selectedTopFolder\?\.folderName|selectedTopFolderColorStyle" feature/file/src/main/java/com/linku/fileRepository: LinkYou-2025/LinkU_Android
Length of output: 2061
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- FileViewModel.kt: categoryColorMap declaration ---'
rg -n "categoryColorMap|MutableStateFlow|StateFlow|toCategoryColorStyleMap|folderName" \
feature/file/src/main/java/com/linku/file/FileViewModel.kt \
feature/file/src/main/java/com/linku/file/viewmodel/folder/state/FolderStateViewModel.kt \
data/src/main/java/com/linku/data/util/toCategoryColorStyleMap.kt \
design/src/main/java/com/linku/design/theme/color/CategoryColorStyle.kt
echo
echo '--- FileViewModel.kt (relevant slices) ---'
sed -n '1,220p' feature/file/src/main/java/com/linku/file/FileViewModel.kt
echo
echo '--- FolderStateViewModel.kt (relevant slices) ---'
sed -n '1,220p' feature/file/src/main/java/com/linku/file/viewmodel/folder/state/FolderStateViewModel.kt
echo
echo '--- toCategoryColorStyleMap.kt ---'
sed -n '1,200p' data/src/main/java/com/linku/data/util/toCategoryColorStyleMap.kt
echo
echo '--- CategoryColorStyle.kt ---'
sed -n '1,220p' design/src/main/java/com/linku/design/theme/color/CategoryColorStyle.ktRepository: LinkYou-2025/LinkU_Android
Length of output: 31528
FileScreen.kt: nullable 키를 직접 인덱싱하지 않도록 수정
folderStateViewModel.selectedTopFolder?.folderName은 String?이라 Map<String, CategoryColorStyle>에 바로 넣을 수 없습니다. let으로 null을 먼저 걸러서 기본값으로 떨어지게 바꾸세요.
수정 예시
- val selectedTopFolderColorStyle =
- categoryColorMap[folderStateViewModel.selectedTopFolder?.folderName]
- ?: CategoryColorStyle.categoryStyleList[0]
+ val selectedTopFolderColorStyle =
+ folderStateViewModel.selectedTopFolder?.folderName?.let { categoryColorMap[it] }
+ ?: CategoryColorStyle.categoryStyleList[0]📝 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.
| val categoryColorMap by fileViewModel.categoryColorMap.collectAsStateWithLifecycle() | |
| val parentFolders by fileViewModel.parentFolders.collectAsStateWithLifecycle() | |
| val subFolders by fileViewModel.subFolders.collectAsStateWithLifecycle() | |
| val links by fileViewModel.links.collectAsStateWithLifecycle() | |
| val notCategorizationLinks by fileViewModel.notCategorizationLinks.collectAsStateWithLifecycle() | |
| val selectedTopFolderColorStyle = | |
| categoryColorMap[folderStateViewModel.selectedTopFolder?.folderName] | |
| ?: CategoryColorStyle.categoryStyleList[0] | |
| val categoryColorMap by fileViewModel.categoryColorMap.collectAsStateWithLifecycle() | |
| val parentFolders by fileViewModel.parentFolders.collectAsStateWithLifecycle() | |
| val subFolders by fileViewModel.subFolders.collectAsStateWithLifecycle() | |
| val links by fileViewModel.links.collectAsStateWithLifecycle() | |
| val notCategorizationLinks by fileViewModel.notCategorizationLinks.collectAsStateWithLifecycle() | |
| val selectedTopFolderColorStyle = | |
| folderStateViewModel.selectedTopFolder?.folderName?.let { categoryColorMap[it] } | |
| ?: CategoryColorStyle.categoryStyleList[0] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@feature/file/src/main/java/com/linku/file/FileScreen.kt` around lines 83 -
90, The selected top folder color lookup in FileScreen is indexing
categoryColorMap with a nullable folder name, which should be handled safely.
Update the selectedTopFolderColorStyle computation near the
collectAsStateWithLifecycle state reads to first unwrap
folderStateViewModel.selectedTopFolder?.folderName with let, then look up the
color in categoryColorMap, and fall back to
CategoryColorStyle.categoryStyleList[0] when the folder name is null or missing.
| val selectedTopFolderColorStyle = | ||
| categoryColorMap[folderStateViewModel.selectedTopFolder?.folderName] | ||
| ?: CategoryColorStyle.categoryStyleList[0] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the referenced file and related color-style definitions/usages.
git ls-files | rg '(^|/)(FileScreen\.kt|CategoryGrid\.kt|MyFoldersGrid\.kt|CategoryColorStyle\.(kt|kts))$' || true
echo '--- FileScreen.kt ---'
sed -n '1,160p' feature/file/src/main/java/com/linku/file/FileScreen.kt
echo '--- CategoryColorStyle files/usages ---'
rg -n "object CategoryColorStyle|class CategoryColorStyle|DEFAULT|categoryStyleList|categoryStyleList\\[0\\]" -S .
echo '--- CategoryGrid.kt ---'
sed -n '1,220p' feature/**/CategoryGrid.kt 2>/dev/null || true
echo '--- MyFoldersGrid.kt ---'
sed -n '350,430p' feature/**/MyFoldersGrid.kt 2>/dev/null || trueRepository: LinkYou-2025/LinkU_Android
Length of output: 12484
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,140p' design/src/main/java/com/linku/design/theme/color/CategoryColorStyle.kt
echo '--- CategoryGrid.kt ---'
sed -n '1,160p' feature/file/src/main/java/com/linku/file/ui/content/CategoryGrid.kt
echo '--- MyFoldersGrid.kt ---'
sed -n '360,420p' feature/file/src/main/java/com/linku/file/ui/content/MyFoldersGrid.ktRepository: LinkYou-2025/LinkU_Android
Length of output: 11294
기본 색상 스타일 소스를 하나로 통일하세요.
FileScreen.kt은 CategoryColorStyle.categoryStyleList[0]을 쓰고, 다른 화면은 CategoryColorStyle.DEFAULT를 씁니다. 두 값은 같지 않아서 기본 카드 색상이 화면마다 달라집니다. 한 쪽으로 통일하는 편이 좋습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@feature/file/src/main/java/com/linku/file/FileScreen.kt` around lines 88 -
90, Default category color handling is inconsistent in FileScreen; it falls back
to CategoryColorStyle.categoryStyleList[0] while other screens use
CategoryColorStyle.DEFAULT. Update the selectedTopFolderColorStyle fallback in
FileScreen.kt to use the same default source as the other screens, so the color
behavior is consistent across the app.
| MyFoldersGrid( | ||
| folders = subFolders, | ||
| notCategorizationLinks = notCategorizationLinks, | ||
| selectedTopFolderColorStyle = selectedTopFolderColorStyle, | ||
| isEditMode = editStateViewModel.isEditMode, | ||
| onAddFolderClick = { | ||
| folderStateViewModel.updateNewFolderBottomSheetVisible(true) | ||
| }, | ||
| onFolderClick = { folder -> | ||
| fileViewModel.getLinks(folder.folderId) | ||
| folderStateViewModel.updateSelectedBottomFolder(folder) | ||
| folderStateViewModel.updateFolderState(FolderState.LINKS) | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant symbols and files
rg -n "_subFoldersCursor|getFoldersAndNotCategorizationLinks|getLinks\\(|getLinksFolders|subFoldersCursor" feature/file/src/main/java
# Show file list for targeted inspection
git ls-files feature/file/src/main/java/com/linku/file
# If FileViewModel exists, inspect its outline first
fd -a "FileViewModel.*" feature/file/src/main/javaRepository: LinkYou-2025/LinkU_Android
Length of output: 4693
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant FileViewModel sections with line numbers
sed -n '430,540p' feature/file/src/main/java/com/linku/file/FileViewModel.kt | cat -n
# Find the repository definition and implementation for getLinksFolders
rg -n "getLinksFolders\\(" feature -g '!**/build/**'Repository: LinkYou-2025/LinkU_Android
Length of output: 4750
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the repository interface/implementation for getLinksFolders
rg -n "getLinksFolders\\s*\\(" feature/file/src/main/java
# Inspect the repository implementation if found
# (replace the path only if rg reports a concrete file path)Repository: LinkYou-2025/LinkU_Android
Length of output: 442
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search the entire repository for the repository type and method definition
rg -n "interface .*FolderRepository|class .*FolderRepository|object .*FolderRepository|fun getLinksFolders\\s*\\(" .
# Narrow down likely data-layer files if the above is too broad
find . -type f \( -name '*Repository*.kt' -o -name '*Repository*.kts' -o -name '*Repository*.java' \) | sed 's#^\./##' | sortRepository: LinkYou-2025/LinkU_Android
Length of output: 2871
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the repository contract
sed -n '1,120p' core/src/main/java/com/linku/core/repository/FolderRepository.kt | cat -n
# Read the implementation around getLinksFolders
sed -n '120,220p' data/src/main/java/com/linku/data/implementation/repository/FolderRepositoryImpl.kt | cat -n
# Read the API contract around getLinksFolders
sed -n '1,120p' data/src/main/java/com/linku/data/api/FolderApi.kt | cat -nRepository: LinkYou-2025/LinkU_Android
Length of output: 12280
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find every assignment/use of the shared cursor state
rg -n "_subFoldersCursor\\.value\\s*=|subFoldersCursor" feature/file/src/main/java core/src/main/java data/src/main/java
# Also inspect the top-folder click path around line 141 in FileScreen
sed -n '128,150p' feature/file/src/main/java/com/linku/file/FileScreen.kt | cat -nRepository: LinkYou-2025/LinkU_Android
Length of output: 2520
폴더 전환 시 _subFoldersCursor를 초기화/분리하세요.
getFoldersAndNotCategorizationLinks()와 getLinks()가 같은 _subFoldersCursor를 재사용해서 서로 다른 folderId 사이에 다음 페이지 커서가 섞입니다. 폴더 컨텍스트가 바뀔 때는 커서를 null로 리셋하거나 folderId별로 분리해야 잘못된 페이지/빈 결과를 막을 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@feature/file/src/main/java/com/linku/file/FileScreen.kt` around lines 166 -
178, The paging cursor used by FileViewModel for subfolders is being shared
across different folder contexts, causing
`getFoldersAndNotCategorizationLinks()` and `getLinks()` to interfere with each
other. Update the `FileScreen` flow where `onFolderClick` switches folders and
the related `FileViewModel` methods so `_subFoldersCursor` is reset to null or
maintained per `folderId` whenever the selected folder changes. Ensure
`getLinks()`, `getFoldersAndNotCategorizationLinks()`, and any cursor update
logic in `FileViewModel` no longer reuse the same cursor across different
folders.
| CategoryItemLayout( | ||
| modifier = Modifier.fillMaxSize(201.10968f/412f), | ||
| colorStyle = categoryColorStyle?:CategoryColorStyle.categoryStyleList[0], | ||
| folder = selectedTopFolder!! | ||
| ) { } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
공유 미리보기 카드의 북마크 아이콘이 클릭해도 아무 동작을 하지 않음.
CategoryItemLayout의 visibleBookmarked(기본 true)와 isEditMode(기본 false)를 명시하지 않아, 이 카드는 북마크 별 아이콘을 노출하면서 onBookmark 콜백은 빈 람다({ })입니다. 사용자가 북마크를 탭해도 반응이 없어 혼란을 줄 수 있습니다. 이 화면은 폴더 공유 미리보기 용도이므로 visibleBookmarked = false로 아이콘 영역을 숨기는 것이 의도에 더 맞아 보입니다.
🛠️ 제안하는 수정
CategoryItemLayout(
modifier = Modifier.fillMaxSize(201.10968f/412f),
colorStyle = categoryColorStyle?:CategoryColorStyle.categoryStyleList[0],
- folder = selectedTopFolder!!
+ folder = selectedTopFolder!!,
+ visibleBookmarked = false
) { }📝 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.
| CategoryItemLayout( | |
| modifier = Modifier.fillMaxSize(201.10968f/412f), | |
| colorStyle = categoryColorStyle?:CategoryColorStyle.categoryStyleList[0], | |
| folder = selectedTopFolder!! | |
| ) { } | |
| } | |
| CategoryItemLayout( | |
| modifier = Modifier.fillMaxSize(201.10968f/412f), | |
| colorStyle = categoryColorStyle?:CategoryColorStyle.categoryStyleList[0], | |
| folder = selectedTopFolder!!, | |
| visibleBookmarked = false | |
| ) { } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@feature/file/src/main/java/com/linku/file/ui/bottom/sheet/ShareBottomSheet.kt`
around lines 278 - 283, In ShareBottomSheet’s CategoryItemLayout preview card,
the bookmark icon is shown even though its onBookmark handler is a no-op, which
makes the UI look interactive but do nothing; update this call site to hide the
bookmark area for this share preview by setting visibleBookmarked to false, and
keep the existing folder/category setup intact so the card remains a read-only
preview.
| items(folders) { folder -> | ||
| MyFolderItem( | ||
| folder = folder, | ||
| colorStyle = selectedTopFolderColorStyle, | ||
| isEditMode = isEditMode, | ||
| onClick = { | ||
| // 일반 모드에서 하위 폴더를 눌렀을 때의 화면 전환/데이터 로딩은 상위로 위임합니다. | ||
| onFolderClick(folder) | ||
| }, | ||
| onEdit = { | ||
| // 편집 아이콘 클릭 시 수정 대상 지정과 바텀시트 노출을 상위로 위임합니다. | ||
| onFolderEditClick(folder) | ||
| }, | ||
| onChangeSharing = { | ||
| // 공유 상태 아이콘 클릭 시 공유 상태 변경 동작을 상위로 위임합니다. | ||
| onChangeSharingClick(folder) | ||
| }, | ||
| onDelete = { | ||
| // 폴더 삭제 확인 후 실행할 실제 삭제 동작을 상위로 위임합니다. | ||
| onDeleteFolder(folder) | ||
| } | ||
| ) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
items(folders)에 key 부재 + MyFolderItem의 로컬 deleteModalWindowVisible 상태 조합이 잘못된 아이템에 상태가 재사용될 위험을 만듭니다.
MyFolderItem은 각 폴더 아이템마다 remember { mutableStateOf(false) }로 삭제 확인 모달 상태를 로컬로 갖습니다(L329). items(folders)에 key가 없으면 리스트가 변경(삭제, 순서 변경 등)될 때 Compose는 위치 기준으로 컴포지션 슬롯을 재사용하므로, 삭제 후 같은 위치로 이동한 다른 폴더가 이전 폴더의 deleteModalWindowVisible=true 상태를 그대로 이어받아 사용자가 누르지 않은 삭제 확인 모달이 갑자기 나타날 수 있습니다. folder.folderId를 key로 지정해 아이템 정체성을 고정하는 것을 권장합니다.
🛠️ 제안하는 수정
- items(folders) { folder ->
+ items(folders, key = { it.folderId }) { folder ->As per coding guidelines, "Focus on state management (state hoisting, remember vs rememberSaveable) in Android/Kotlin/Compose code".
Also applies to: 313-358
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@feature/file/src/main/java/com/linku/file/ui/content/MyFoldersGrid.kt` around
lines 162 - 184, `items(folders)` is missing a stable key, so Compose may reuse
the wrong slot state and carry `MyFolderItem`’s local `deleteModalWindowVisible`
across folder changes. Update the `items` call in `MyFoldersGrid` to use a
stable identifier such as `folder.folderId` as the key so each folder keeps its
own composition state. This will prevent the delete confirmation modal inside
`MyFolderItem` from appearing on the wrong folder after deletions or reordering.
Source: Coding guidelines
KateteDeveloper
left a comment
KateteDeveloper
left a comment
There was a problem hiding this comment.
🤖 Claude Code 리뷰 (/review 145, medium effort)
확정된 버그 2건과 정리성 제안 1건을 남깁니다.
diff 범위 밖이라 인라인으로 못 남긴 항목 1건:
feature/file/src/main/java/com/linku/file/ui/bottom/sheet/_ShareBottomSheet.kt:411 — Folder가 이제 Category를 상속하도록 바뀌었는데(같은 파일 184번 줄 부근), when (selectDepth) { is Category -> ...; is Folder -> ... } 분기(405~420번 줄, 이번 PR에서 수정되지 않은 영역)에서 is Category가 is Folder보다 먼저 검사됩니다. Kotlin when은 첫 매치를 사용하므로 selectDepth가 실제로 Folder여도 is Category 분기가 먼저 매치되어 is Folder 분기(414번 줄)는 도달 불가능해집니다.
재현 시나리오: 사용자가 카테고리와 그 안의 특정 폴더까지 모두 선택해 공유하려 하면 selectDepth가 Folder 인스턴스가 되지만, 메뉴 텍스트는 is Category 분기로 빠져 카테고리 이름만 표시되고 "카테고리 > 폴더" 형식이 나오지 않습니다. is Folder 분기를 is Category보다 먼저 배치해주세요.
| @@ -155,9 +202,18 @@ fun FileScreen( | |||
| } | |||
| } | |||
| FolderState.LINKS -> { | |||
There was a problem hiding this comment.
[Bug, CONFIRMED] FolderState.LINKS 분기는 isSharedFolders 여부와 무관하게 항상 ClassifiedLinksGrid를 렌더링합니다. TOP/BOTTOM 상태는 if (!folderStateViewModel.isSharedFolders)로 분기하는데(133, 165번 줄), LINKS 상태만 이 분기가 빠져 있습니다.
삭제된 LinksGrid는 내부적으로 isShareMode를 계산해 공유 모드에서는 "링크 추가하기" 셀을 숨기고 long-press 삭제를 막았는데, 이 동작이 ClassifiedLinksGrid에는 재구현되어 있지 않습니다(해당 컴포저블에는 공유 모드 파라미터 자체가 없음).
재현 시나리오: SharedBottomFolders.kt:71에서 공유받은 폴더를 클릭하면 FolderState.LINKS로 전환되는데, 이때도 "링크 추가하기" placeholder가 노출되고 링크를 long-press하면 fileViewModel.deleteLink(linkId)가 호출되어 공유받은 폴더 안의 링크를 삭제할 수 있게 됩니다(기존 코드는 이를 명시적으로 막고 있었음).
제안: ClassifiedLinksGrid에 isSharedFolders 관련 파라미터를 추가하거나, 공유 모드에서는 별도 컴포저블(SharedLinksGrid 등)로 분기 처리해주세요.
|
|
||
| folderRepository.deleteSubfolder(folderId) | ||
| _subFolders.value = _subFolders.value.filterIndexed { i, _ -> i != index } | ||
| _subFolders.value = _subFolders.value.filterIndexed { i, folder -> folder.folderId != folderId } |
There was a problem hiding this comment.
[Cleanup, minor] filterIndexed { i, folder -> folder.folderId != folderId }에서 인덱스 i를 사용하지 않습니다. filter { it.folderId != folderId }로 단순화할 수 있습니다.
| ) | ||
| @Composable | ||
| private fun CategoryGridTest(){ | ||
| /** Preview에서는 외부 ViewModel 없이 빈 상태의 그리드 레이아웃만 확인합니다. */ |
There was a problem hiding this comment.
이러면 프리뷰의 의미가 없지 않나요...?
There was a problem hiding this comment.
그렇긴 해ㅋㅋㅋ 폰으로만 봐서 그냥 넘겼었네..
| modifier: Modifier = Modifier, | ||
| contentPadding: PaddingValues = PaddingValues(top = 20.dp, start = 20.dp, end = 20.dp, bottom = 60.dp), | ||
| categories: List<FolderSimpleInfo>, | ||
| categoryColorMap: Map<String, CategoryColorStyle>, |
There was a problem hiding this comment.
이거 설계 진짜 개발에 미친 것 같다
와 toCategoryColorStyleMap 여기서 선형 탐색까지 고려했네 와 이건 쫌 인정
There was a problem hiding this comment.
고려 안했는데ㅋㅋㅋㅋ Map 말하는 건지 List 말하는 건지 몰겠지만 둘 다 선형 탐색 안 쓸 거야
There was a problem hiding this comment.
아 뭐야 Map 잘 써서 칭찬했는데
| } | ||
| }, | ||
| // 폴더 이름으로 매핑된 카테고리 색상이 없으면 기본 색상 스타일을 사용합니다. | ||
| colorStyle = categoryColorMap[folder.folderName] ?: CategoryColorStyle.DEFAULT, |
There was a problem hiding this comment.
폴더 id가 아니라 name인 이유가 뭐야??(그냥 궁금증)
There was a problem hiding this comment.
id는 유저마다 다 달라서 이름으로 찾아
| @Preview(showBackground = true) | ||
| @Composable | ||
| private fun ClassifiedLinksGridTest(){ | ||
| ClassifiedLinksGrid( |
There was a problem hiding this comment.
아 제가 아마 paperLogy 프리뷰에서도 보게 하려고 LinkUPreview라는 것을 만들었는데요. 뭐 안 써도 무방합니다. 그냥 그렇다고요
There was a problem hiding this comment.
폰으로만 봐서 생각을 안했어..
|
|
||
| // 분류되지 않은 링크가 없을 때 링크 추가 동선을 안내하는 모달창입니다. | ||
| ModalWindow( | ||
| visible = categorizationModalWindowVisible, |
There was a problem hiding this comment.
하지만 그것이 변수의 역할인 걸
| /** 현재 카드 너비가 디자인 기준 너비에 비해 얼마나 커지거나 작아졌는지 나타냅니다. */ | ||
| val scaleW = maxWidth / baseWidth | ||
|
|
||
| /** 현재 카드 높이가 디자인 기준 높이에 비해 얼마나 커지거나 작아졌는지 나타냅니다. */ | ||
| val scaleH = maxHeight / baseHeight | ||
|
|
||
| /** 너비/높이 중 더 작은 스케일을 사용해 내부 요소가 카드 밖으로 넘치지 않게 합니다. */ | ||
| val scale = minOf(scaleW, scaleH) | ||
|
|
||
| /** 디자인 기준 dp 값을 현재 카드 크기에 맞는 dp 값으로 변환합니다. */ | ||
| fun s(dp: Dp) = dp * scale | ||
|
|
||
| /** 디자인 기준 sp 값을 현재 카드 크기에 맞는 sp 값으로 변환합니다. */ | ||
| fun ssp(textUnit: TextUnit) = (textUnit.value * scale).sp |
There was a problem hiding this comment.
이거 어디서 많이 계속 보이는데 그냥 공통 유틸로 빼면 안돼?? 안되려나 아니 자꾸 똑같은게 등장하는 느낌적인 느낌이야...
There was a problem hiding this comment.
아 이것도
/** 2열 고정 그리드로 카테고리 목록을 표시. */
LazyVerticalGrid(
modifier = Modifier.fillMaxSize(),
contentPadding = contentPadding,
columns = GridCells.Fixed(2),
verticalArrangement = Arrangement.spacedBy(INTER_LAYER_PADDING.dp),
horizontalArrangement = Arrangement.spacedBy(horizontalSpacing)
) {뭔가가 뭔가 계속 등장 하는 느낌이야..
|
|
||
| /** 링크 썸네일을 Coil로 로드하고, 실패/빈 값일 때 기본 이미지를 표시하도록 설정합니다. */ | ||
| val painter = ImageRequest.Builder(LocalContext.current) | ||
| .data(link?.linkuImageUrl) // url이 null일 수도 있음 |
There was a problem hiding this comment.
이런 무서운 일이 일어나면 안돼.....
There was a problem hiding this comment.
remember(link?.linkuImageUrl) 이렇게 만들어서 링크가 바뀔때만 리컴포지션 일어나게 하는 건 어때?
| * @param tag 칩 내부에 표시할 태그명입니다. | ||
| */ | ||
| @Composable | ||
| fun LinkItemTag(tag: String) { |
There was a problem hiding this comment.
이거 최상위로 빼면 더 좋지 않을까?
| * @param rotation 레이어에 적용할 회전 각도입니다. | ||
| */ | ||
| @Composable | ||
| fun FolderLayerBox( |
There was a problem hiding this comment.
이거 최상위로 따로 빼는게 좋지 않을까??
| @@ -27,21 +27,23 @@ import androidx.compose.ui.platform.LocalLayoutDirection | |||
| import androidx.compose.ui.tooling.preview.Preview | |||
| import androidx.compose.ui.unit.dp | |||
| import androidx.hilt.navigation.compose.hiltViewModel | |||
There was a problem hiding this comment.
deprecated된 거 말고 import문 바꿔주세용
|
수고 많으셨습니다~! |
2 participants
📝 설명
✔️ PR 유형
어떤 변경 사항이 있나요?
📎 관련 이슈 번호
Summary by CodeRabbit
릴리스 노트