feat(home): implement sorting functionality for coin list#51
feat(home): implement sorting functionality for coin list#51
Conversation
There was a problem hiding this comment.
Pull request overview
Implements interactive sorting for the home coin list and aligns the sort header UI with the new behavior.
Changes:
- Home state/contract: adds
sortField,sortDirection, andOnSortClickedtoHomeUiState/HomeEventso sorting can be driven from the ViewModel. - ViewModel logic: wires
OnSortClickedintoHomeViewModel, introducessortCoins, and ensures incoming coin data is always sorted according to the current sort field/direction. - UI wiring: connects
SortHeaderinHomeScreento the new sort state and events, and adjustsSortHeaderItemtext size for the sort header labels.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| home/presentation/src/main/java/io/soma/cryptobook/home/presentation/component/sortheader/SortHeaderItem.kt | Adjusts sort header label text size (14sp → 12sp) to match updated design. |
| home/presentation/src/main/java/io/soma/cryptobook/home/presentation/HomeViewModel.kt | Adds sort handling (OnSortClicked, updateSort, sortCoins, default/toggle direction) and applies sorting to observed coin lists. |
| home/presentation/src/main/java/io/soma/cryptobook/home/presentation/HomeScreen.kt | Binds sort state from HomeUiState into SortHeader and dispatches OnSortClicked events on header clicks. |
| home/presentation/src/main/java/io/soma/cryptobook/home/presentation/HomeContract.kt | Extends HomeUiState with sort-related fields, introduces SortField enum, and adds OnSortClicked event. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return if (sortDirection == SortDirection.Desc) { | ||
| coins.sortedWith(comparator).asReversed() | ||
| }else{ | ||
| coins.sortedWith(comparator) |
There was a problem hiding this comment.
This if/else branch uses a }else{ style that differs from the rest of the Kotlin codebase, which consistently uses spaces (} else {, e.g. core/network/src/main/java/io/soma/cryptobook/core/network/base/BaseDataSource.kt:38). For consistency with established formatting conventions (and to keep tools like Spotless/ktlint happy), please add spaces around else here.
| val coins: List<CoinItem> = emptyList(), | ||
| val isLoading: Boolean = false, | ||
| val errorMsg: String? = null, | ||
| val sortField: SortField = SortField.Price, | ||
| val sortDirection: SortDirection = SortDirection.Desc, |
There was a problem hiding this comment.
HomeUiState now exposes sortDirection using SortDirection from the component.sortheader package, which couples the screen contract to a specific UI component implementation. In other screens (e.g. coin-detail/presentation/src/main/java/io/soma/cryptobook/coindetail/presentation/CoinDetailContract.kt), UI state types stay independent of leaf UI components, so it would be more consistent and maintainable to define SortDirection alongside HomeUiState (or in a shared presentation package) and let SortHeader/SortHeaderItem depend on that instead.
| fontFamily = fontFamily, | ||
| fontWeight = FontWeight.Bold, | ||
| fontSize = 14.sp, | ||
| fontSize = 12.sp, |
There was a problem hiding this comment.
lineHeight = 20.sp 이기 때문에 fontSize를 바꾸는 것은 의미가 없습니다. 바꾼 이유가 있을까요?
There was a problem hiding this comment.
24h change % 부분이 가로로 길이가 초과되서요
| onSymbolClick = { onEvent(HomeEvent.OnSortClicked(SortField.Symbol)) }, | ||
| onPriceClick = { onEvent(HomeEvent.OnSortClicked(SortField.Price)) }, | ||
| onChangeClick = { onEvent(HomeEvent.OnSortClicked(SortField.Change)) }, |
There was a problem hiding this comment.
명시적으로 넣는거보다 state.sortField를 인자로 넣어도 되겠네요
관련 이슈
사전 점검
./gradlew spotlessApply실행을 완료하였습니다.작업 개요
상세 내용
1️⃣ UI/UX 변경
2️⃣ 로직 및 아키텍처
3️⃣ 리팩토링
리뷰어 집중 포인트(참고 사항)
후속 작업