[feat] Feed UX 개선 - 카테고리 필터, 이미지 피커, 툴팁 등#88
Open
DongChyeon wants to merge 39 commits intodevelopfrom
Open
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…로 이관 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
DongChyeon
changed the title
Walkthrough이 PR은 피드 시스템을 다중 이미지 지원, 제목/링크 메타데이터, 클라이언트 측 카테고리 필터링, 정렬 기능을 추가하여 고도화합니다. API v2 마이그레이션, 데이터 모델 재구성, FeedCard 및 HomeScreen 전반적인 UI 변경, 업로드 플로우 개선을 포함합니다. Changes
Sequence DiagramsequenceDiagram
actor User
participant HomeUI as HomeScreen<br/>(UI)
participant HomeVM as HomeViewModel
participant FeedRepo as FeedRepository
participant FeedAPI as FeedApiService
participant WebNav as WebView<br/>Navigation
User->>HomeUI: 피드 화면 로드
HomeUI->>HomeVM: 초기 로드 요청
HomeVM->>FeedRepo: getFeedList() 호출
FeedRepo->>FeedAPI: GET /api/v2/feeds 요청
FeedAPI-->>FeedRepo: FeedList 응답 (다중 이미지 포함)
FeedRepo-->>HomeVM: 응답 맵핑 (allFeeds 설정)
HomeVM->>HomeVM: applyCategories() 필터링<br/>(selectedCategories 적용)
HomeVM-->>HomeUI: uiState 업데이트<br/>(feeds 렌더링)
HomeUI->>User: 피드 목록 표시<br/>(이미지 캐러셀, 제목, 링크 버튼)
User->>HomeUI: 카테고리 칩 선택
HomeUI->>HomeVM: OnCategoryToggled(category) 인텐트
HomeVM->>HomeVM: selectedCategories 토글<br/>applyCategories() 재적용
HomeVM-->>HomeUI: 필터된 feeds 업데이트
HomeUI->>User: 필터된 피드 목록 갱신
User->>HomeUI: 제목/이미지 내 링크 클릭
HomeUI->>HomeUI: onLinkClick(url) 콜백 호출
HomeUI->>WebNav: navigateToWebView(title, url)
WebNav-->>User: WebView 화면 네비게이션
User->>HomeUI: 정렬 버튼 클릭
HomeUI->>HomeUI: OptionSheet 표시 (showSortSheet=true)
User->>HomeUI: 정렬 옵션 선택 (FilterChip)
HomeUI->>HomeVM: OnFilterSelected(filterChip) 인텐트
HomeVM->>HomeVM: selectedFilter 업데이트<br/>applyCategories() 재적용
HomeVM-->>HomeUI: 정렬된 feeds 업데이트
HomeUI->>User: 정렬된 피드 목록 갱신
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/ui/src/main/java/com/sseotdabwa/buyornot/core/ui/webview/WebViewNavigation.kt (1)
17-23:⚠️ Potential issue | 🟡 Minor
title파라미터도 URL 인코딩이 필요합니다.라인 22에서
url만 인코딩하고title은 raw 문자열로 전달되어,title에&,?,=같은 예약 문자가 포함되면 라우트 파싱이 깨질 수 있습니다. Jetpack Navigation Compose는 query parameter 값의 reserved character를 인코딩해야 하며, 디코딩도 함께 처리해야 합니다.수정 제안
fun NavController.navigateToWebView( title: String, url: String, ) { + val encodedTitle = URLEncoder.encode(title, "UTF-8") val encodedUrl = URLEncoder.encode(url, "UTF-8") - this.navigate("$WEBVIEW_ROUTE?title=$title&url=$encodedUrl") + this.navigate("$WEBVIEW_ROUTE?title=$encodedTitle&url=$encodedUrl") }- val title = backStackEntry.arguments?.getString("title") ?: "" + val title = URLDecoder.decode(backStackEntry.arguments?.getString("title") ?: "", "UTF-8")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/ui/src/main/java/com/sseotdabwa/buyornot/core/ui/webview/WebViewNavigation.kt` around lines 17 - 23, The navigateToWebView function currently only URL-encodes url, leaving title unencoded which can break route parsing if title contains reserved characters; update navigateToWebView to also URLEncoder.encode the title (e.g., encodedTitle = URLEncoder.encode(title, "UTF-8")) and use that encodedTitle when building the route string (alongside encodedUrl), and ensure receiving destination decodes both query parameters as needed.
🧹 Nitpick comments (6)
core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/components/FeedCard.kt (1)
649-650: 툴팁 문구는 문자열 리소스로 빼는 편이 좋습니다.새 user-facing 문구가 디자인시스템 코드에 하드코딩되어 있으면 추후 문구 수정과 로컬라이징 대응이 번거로워집니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/components/FeedCard.kt` around lines 649 - 650, The Text composable in FeedCard.kt is hardcoding a user-facing string; extract "상품 링크를 확인해보세요!" into the Android string resources and use stringResource(...) in the Text call instead. Add a new entry (e.g., name="feed_card_check_product_link") with the Korean text to your strings.xml (and localized variants as needed), update the Text invocation in FeedCard.kt to reference stringResource(R.string.feed_card_check_product_link), and ensure you import androidx.compose.ui.res.stringResource; keep the original semantics and formatting.core/network/src/main/java/com/sseotdabwa/buyornot/core/network/api/FeedApiService.kt (1)
27-32: KDoc에category파라미터 설명도 같이 업데이트해 주세요.Line 38에서 시그니처는 확장됐지만, 상단 주석에는
category설명이 없어 유지보수 시 혼동 여지가 있습니다.Also applies to: 38-38
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/network/src/main/java/com/sseotdabwa/buyornot/core/network/api/FeedApiService.kt` around lines 27 - 32, Update the KDoc for the FeedApiService method that was extended to accept a category parameter (the method signature in FeedApiService.kt now includes category) by adding an `@param` category entry describing its purpose, allowed values/format (e.g., category id or name), and behavior when omitted (e.g., returns all categories); ensure the new `@param` line is consistent with existing `@param` style and placed alongside cursor/size/feedStatus entries in the method's KDoc.docs/superpowers/specs/2026-04-13-filter-sort-design.md (1)
51-53: 펜스 코드 블록에 언어 지정 누락마크다운 린터가 펜스 코드 블록에 언어가 지정되지 않았음을 경고하고 있습니다. ASCII 레이아웃 다이어그램이라면
text또는plaintext를 사용하세요.📝 제안된 수정
-``` +```text [ 정렬아이콘(ic_sort) ] [ 패션∙잡화 ] [ 명품∙프리미엄 ] [ 화장품∙뷰티 ] ...</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/superpowers/specs/2026-04-13-filter-sort-design.mdaround lines 51 -
53, The fenced code block containing the ASCII layout ("[ 정렬아이콘(ic_sort) ] [
패션∙잡화 ] ...") is missing a language tag; update the fence to include a plain
text language (e.g., use "text" or "plaintext") so the markdown linter stops
warning. Locate the code fence in the spec file (the block shown in the diff)
and prepend the opening backticks with the language token (for exampletext) and keep the closingunchanged.</details> </blockquote></details> <details> <summary>feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt (1)</summary><blockquote> `559-561`: **displayName 문자열 비교 대신 enum 직접 비교 권장** `displayName` 문자열 비교는 향후 표시명이 변경되거나 중복될 경우 필터링이 실패할 수 있습니다. `FeedItem`에 `FeedCategory` enum 자체를 저장하고 enum identity로 비교하는 것이 더 안전합니다. 현재 구조상 `FeedItem.category`가 `String`이므로, 역방향 조회가 필요하다면 `FeedCategory.entries.find { it.displayName == feed.category }`로 먼저 변환 후 비교하거나, `FeedItem`에 `FeedCategory` 타입 필드를 추가하는 것을 고려해주세요. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt` around lines 559 - 561, The filter currently compares FeedItem.category (a String) against FeedCategory.displayName strings which is fragile; change to compare enum identities by either converting the string to the enum first (use FeedCategory.entries.find { it.displayName == feed.category } and then compare to the selected categories) or, better, update FeedItem to hold a FeedCategory field and filter directly (e.g., feeds.filter { feed -> categories.any { it == feed.category } }). Ensure references to FeedItem.category and FeedCategory.entries/find are updated accordingly. ``` </details> </blockquote></details> <details> <summary>feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeScreen.kt (2)</summary><blockquote> `349-353`: **툴팁 상태가 피드 변경 시 리셋되지 않음** `showLinkTooltip`이 `true`로 초기화된 후 한 번 dismiss되면 `filteredFeeds`가 변경되어도 다시 표시되지 않습니다. 새로고침이나 탭 전환 후에도 툴팁이 다시 표시되어야 한다면, `remember` 키에 관련 상태를 추가하거나 별도 로직이 필요할 수 있습니다. 현재 동작이 의도된 것이라면 무시해도 됩니다. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeScreen.kt` around lines 349 - 353, The tooltip visibility (showLinkTooltip) is only remembered once and won't reset when filteredFeeds changes; update the state so it resets on feed changes by including filteredFeeds in the remember key or by adding a small effect that sets showLinkTooltip = true when filteredFeeds changes (mirror the existing remember(filteredFeeds) usage used for tooltipTargetIndex) so the tooltip reappears after refresh/tab change; adjust the declaration/initialization of showLinkTooltip in HomeScreen.kt accordingly and reference showLinkTooltip, filteredFeeds and tooltipTargetIndex when making the change. ``` </details> --- `546-559`: **scrollToCenter에서 아이템이 아직 레이아웃되지 않은 경우 처리** `scrollToCenter` 함수는 `visibleItemsInfo`에서 해당 인덱스를 찾지 못하면 `animateScrollToItem(index)`로 폴백합니다. 이 접근은 합리적이지만, 첫 번째 스크롤 후 아이템이 중앙에 오지 않을 수 있습니다. 필요시 스크롤 완료 후 다시 중앙 정렬을 시도하는 로직을 추가할 수 있습니다. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeScreen.kt` around lines 546 - 559, scrollToCenter currently falls back to listState.animateScrollToItem(index) when the target isn't in visibleItemsInfo, but that can leave the item off-center after layout; modify scrollToCenter so that after calling listState.animateScrollToItem(index) you wait for the new layout (e.g., suspend briefly or read listState.layoutInfo again) and then compute the item's center using listState.layoutInfo.visibleItemsInfo and call listState.animateScrollBy(...) to center it; implement one retry (or small delay) if the item still isn't found to ensure the item is properly centered after layout changes. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In
@core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/components/FeedCard.kt:
- Around line 315-329: The bug is that currentAspectRatio (from
imageAspectRatios.getOrElse(pagerState.currentPage)) is reused for every page,
causing non-active pages to render at the wrong aspect; change the
HorizontalPager item to use a page-specific ratio by calling
imageAspectRatios.getOrElse(page) inside the pager lambda for the inner Box, and
if you want the overall card container to animate by the current page height
keep a separate computed containerAspectRatio (based on pagerState.currentPage)
applied to the outer Box (with animateContentSize) while each page's inner Box
uses its own aspectRatio retrieved by page; update references to
currentAspectRatio, imageAspectRatios, pagerState, and the Box inside the
HorizontalPager accordingly.- Around line 368-379: The current guard only checks productLink != null which
still allows empty or whitespace strings to render LinkButton and open an
invalid WebView; change the condition to filter out blank links (use
productLink.isNullOrBlank() or !productLink.isNullOrBlank()) so the LinkButton
block (LinkButton, onLinkClick usage) only runs when productLink contains a
non-blank URL (and pass the non-blank string into onLinkClick).In
@core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/shape/TopArrowBubbleShape.kt:
- Around line 41-44: The computed arrowCenterX (currently size.width -
arrowOffsetPx used before moveTo/lineTo) can fall outside the tooltip bounds
when arrowOffsetFromRight or tooltip width varies; clamp arrowCenterX to the
valid range [arrowWidthPx/2, size.width - arrowWidthPx/2] before calling
moveTo/lineTo to ensure the arrow isn't drawn outside the shape. Update the
calculation in TopArrowBubbleShape (where arrowCenterX is defined) to clamp the
value and then use that clamped arrowCenterX in the existing moveTo and lineTo
calls.In
@feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt:
- Around line 174-185: handleCategoryToggled currently calls loadFeeds causing
an unnecessary API call on every toggle; change it to only update
selectedCategories and re-filter locally from the cached feed list instead of
invoking loadFeeds. Specifically, in handleCategoryToggled (and the updateState
block that updates selectedCategories) remove the loadFeeds() call and set the
ViewModel state’s feeds field by filtering state.allFeeds (or the cached
allFeeds collection) according to the new selectedCategories so the UI updates
locally without network requests.In
@feature/upload/src/main/java/com/sseotdabwa/buyornot/feature/upload/ui/UploadViewModel.kt:
- Around line 87-90: The code validates currentState.link but later reads
mutable currentState.title/currentState.link again, so snapshot the inputs at
submission time: in UploadViewModel's submit/upload handler (the method
containing LinkValidator.isValid and the sendSideEffect call) capture val
titleSnapshot = currentState.title and val linkSnapshot = currentState.link
before validating; use linkSnapshot for LinkValidator.isValid and thereafter
pass titleSnapshot and linkSnapshot into the upload request and any side-effects
instead of reading currentState again; apply the same snapshot approach to the
similar block referenced at the area corresponding to lines 120-126.- Around line 49-60: The AddImages handler (UploadIntent.AddImages) fails to
dedupe duplicates inside the incoming batch: compute a batch-unique list first
(e.g., make intent.uris distinct/preserve order) before comparing against
currentState.selectedImageUris and MAX_IMAGE_COUNT; use that batch-unique list
instead of deduped so same-chooser duplicates are removed, then compute toAdd =
batchUnique.filter { it !in existing }. Update hasDuplicates to reflect if
batchUnique.size < intent.uris.size (or if any incoming duplicates were dropped)
and keep overflow logic based on toAdd vs batchUnique, then
updateState(selectedImageUris = ... + toAdd) and send the appropriate
UploadSideEffect.ShowSnackbar messages.
Outside diff comments:
In
@core/ui/src/main/java/com/sseotdabwa/buyornot/core/ui/webview/WebViewNavigation.kt:
- Around line 17-23: The navigateToWebView function currently only URL-encodes
url, leaving title unencoded which can break route parsing if title contains
reserved characters; update navigateToWebView to also URLEncoder.encode the
title (e.g., encodedTitle = URLEncoder.encode(title, "UTF-8")) and use that
encodedTitle when building the route string (alongside encodedUrl), and ensure
receiving destination decodes both query parameters as needed.
Nitpick comments:
In
@core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/components/FeedCard.kt:
- Around line 649-650: The Text composable in FeedCard.kt is hardcoding a
user-facing string; extract "상품 링크를 확인해보세요!" into the Android string resources
and use stringResource(...) in the Text call instead. Add a new entry (e.g.,
name="feed_card_check_product_link") with the Korean text to your strings.xml
(and localized variants as needed), update the Text invocation in FeedCard.kt to
reference stringResource(R.string.feed_card_check_product_link), and ensure you
import androidx.compose.ui.res.stringResource; keep the original semantics and
formatting.In
@core/network/src/main/java/com/sseotdabwa/buyornot/core/network/api/FeedApiService.kt:
- Around line 27-32: Update the KDoc for the FeedApiService method that was
extended to accept a category parameter (the method signature in
FeedApiService.kt now includes category) by adding an@paramcategory entry
describing its purpose, allowed values/format (e.g., category id or name), and
behavior when omitted (e.g., returns all categories); ensure the new@paramline
is consistent with existing@paramstyle and placed alongside
cursor/size/feedStatus entries in the method's KDoc.In
@docs/superpowers/specs/2026-04-13-filter-sort-design.md:
- Around line 51-53: The fenced code block containing the ASCII layout ("[
정렬아이콘(ic_sort) ] [ 패션∙잡화 ] ...") is missing a language tag; update the fence to
include a plain text language (e.g., use "text" or "plaintext") so the markdown
linter stops warning. Locate the code fence in the spec file (the block shown in
the diff) and prepend the opening backticks with the language token (for example
text) and keep the closingunchanged.In
@feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeScreen.kt:
- Around line 349-353: The tooltip visibility (showLinkTooltip) is only
remembered once and won't reset when filteredFeeds changes; update the state so
it resets on feed changes by including filteredFeeds in the remember key or by
adding a small effect that sets showLinkTooltip = true when filteredFeeds
changes (mirror the existing remember(filteredFeeds) usage used for
tooltipTargetIndex) so the tooltip reappears after refresh/tab change; adjust
the declaration/initialization of showLinkTooltip in HomeScreen.kt accordingly
and reference showLinkTooltip, filteredFeeds and tooltipTargetIndex when making
the change.- Around line 546-559: scrollToCenter currently falls back to
listState.animateScrollToItem(index) when the target isn't in visibleItemsInfo,
but that can leave the item off-center after layout; modify scrollToCenter so
that after calling listState.animateScrollToItem(index) you wait for the new
layout (e.g., suspend briefly or read listState.layoutInfo again) and then
compute the item's center using listState.layoutInfo.visibleItemsInfo and call
listState.animateScrollBy(...) to center it; implement one retry (or small
delay) if the item still isn't found to ensure the item is properly centered
after layout changes.In
@feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt:
- Around line 559-561: The filter currently compares FeedItem.category (a
String) against FeedCategory.displayName strings which is fragile; change to
compare enum identities by either converting the string to the enum first (use
FeedCategory.entries.find { it.displayName == feed.category } and then compare
to the selected categories) or, better, update FeedItem to hold a FeedCategory
field and filter directly (e.g., feeds.filter { feed -> categories.any { it ==
feed.category } }). Ensure references to FeedItem.category and
FeedCategory.entries/find are updated accordingly.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yml **Review profile**: CHILL **Plan**: Pro **Run ID**: `e5101cd7-ee28-4d97-95ea-7e8225a64ed2` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 0001563a6d321ff2ea7f6e3f795472455d191f22 and dc1b54804fd30aa9b8fdbcbc473ac199659ac5e4. </details> <details> <summary>⛔ Files ignored due to path filters (3)</summary> * `core/designsystem/src/main/res/drawable-xhdpi/img_my_feed_empty.png` is excluded by `!**/*.png` * `core/designsystem/src/main/res/drawable-xxhdpi/img_my_feed_empty.png` is excluded by `!**/*.png` * `core/designsystem/src/main/res/drawable-xxxhdpi/img_my_feed_empty.png` is excluded by `!**/*.png` </details> <details> <summary>📒 Files selected for processing (30)</summary> * `app/src/main/java/com/sseotdabwa/buyornot/navigation/BuyOrNotNavHost.kt` * `core/data/src/main/java/com/sseotdabwa/buyornot/core/data/repository/FeedRepositoryImpl.kt` * `core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/components/FeedCard.kt` * `core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/components/OptionSheet.kt` * `core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/icon/BuyOrNotIcons.kt` * `core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/icon/BuyOrNotImgs.kt` * `core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/shape/TopArrowBubbleShape.kt` * `core/designsystem/src/main/res/drawable/ic_link.xml` * `core/designsystem/src/main/res/drawable/ic_sort.xml` * `core/network/src/main/java/com/sseotdabwa/buyornot/core/network/api/FeedApiService.kt` * `core/network/src/main/java/com/sseotdabwa/buyornot/core/network/dto/request/FeedRequest.kt` * `core/network/src/main/java/com/sseotdabwa/buyornot/core/network/dto/response/FeedListResponse.kt` * `core/ui/src/main/java/com/sseotdabwa/buyornot/core/ui/webview/WebViewNavigation.kt` * `docs/superpowers/plans/2026-04-13-filter-sort.md` * `docs/superpowers/specs/2026-04-13-filter-sort-design.md` * `domain/src/main/java/com/sseotdabwa/buyornot/domain/model/Feed.kt` * `domain/src/main/java/com/sseotdabwa/buyornot/domain/repository/FeedRepository.kt` * `feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/navigation/HomeNavigation.kt` * `feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeContract.kt` * `feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeScreen.kt` * `feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt` * `feature/notification/src/main/java/com/sseotdabwa/buyornot/feature/notification/navigation/NotificationNavigation.kt` * `feature/notification/src/main/java/com/sseotdabwa/buyornot/feature/notification/ui/NotificationDetailScreen.kt` * `feature/upload/build.gradle.kts` * `feature/upload/src/main/java/com/sseotdabwa/buyornot/feature/upload/navigation/UploadNavigation.kt` * `feature/upload/src/main/java/com/sseotdabwa/buyornot/feature/upload/ui/UploadContract.kt` * `feature/upload/src/main/java/com/sseotdabwa/buyornot/feature/upload/ui/UploadScreen.kt` * `feature/upload/src/main/java/com/sseotdabwa/buyornot/feature/upload/ui/UploadViewModel.kt` * `feature/upload/src/main/java/com/sseotdabwa/buyornot/feature/upload/util/LinkValidator.kt` * `feature/upload/src/test/java/com/sseotdabwa/buyornot/feature/upload/util/LinkValidatorTest.kt` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+315
to
+329
| val currentAspectRatio = imageAspectRatios.getOrElse(pagerState.currentPage) { ImageAspectRatio.SQUARE } | ||
|
|
||
| // 상품 이미지 박스 | ||
| Box(modifier = modifier) { | ||
| HorizontalPager( | ||
| state = pagerState, | ||
| contentPadding = PaddingValues(horizontal = 20.dp), | ||
| pageSpacing = 10.dp, | ||
| modifier = Modifier.animateContentSize(), | ||
| ) { page -> | ||
| Box( | ||
| modifier = | ||
| Modifier | ||
| .fillMaxWidth() | ||
| .aspectRatio(imageAspectRatio.ratio) | ||
| .clip(RoundedCornerShape(16.dp)), | ||
| .aspectRatio(currentAspectRatio.ratio) | ||
| .clip(RoundedCornerShape(16.dp)) |
There was a problem hiding this comment.
페이지별 비율이 아니라 현재 페이지 비율이 모든 이미지에 적용됩니다.
지금은 pagerState.currentPage로 구한 비율을 모든 page에 재사용하고 있어서, 한 피드 안에 1:1과 4:5가 섞여 있으면 비선택 페이지가 잘못된 높이로 렌더링됩니다. 스와이프 중에도 비율이 튀는 문제가 생깁니다.
🔧 제안 수정
- val currentAspectRatio = imageAspectRatios.getOrElse(pagerState.currentPage) { ImageAspectRatio.SQUARE }
-
Box(modifier = modifier) {
HorizontalPager(
state = pagerState,
contentPadding = PaddingValues(horizontal = 20.dp),
pageSpacing = 10.dp,
modifier = Modifier.animateContentSize(),
) { page ->
+ val pageAspectRatio = imageAspectRatios.getOrElse(page) { ImageAspectRatio.SQUARE }
Box(
modifier =
Modifier
.fillMaxWidth()
- .aspectRatio(currentAspectRatio.ratio)
+ .aspectRatio(pageAspectRatio.ratio)
.clip(RoundedCornerShape(16.dp))
.clickable { onFullscreenClick(page) },
) {현재 페이지 높이만 기준으로 카드 컨테이너를 애니메이션하고 싶다면, 컨테이너 높이 계산만 별도로 분리하는 편이 안전합니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| val currentAspectRatio = imageAspectRatios.getOrElse(pagerState.currentPage) { ImageAspectRatio.SQUARE } | |
| // 상품 이미지 박스 | |
| Box(modifier = modifier) { | |
| HorizontalPager( | |
| state = pagerState, | |
| contentPadding = PaddingValues(horizontal = 20.dp), | |
| pageSpacing = 10.dp, | |
| modifier = Modifier.animateContentSize(), | |
| ) { page -> | |
| Box( | |
| modifier = | |
| Modifier | |
| .fillMaxWidth() | |
| .aspectRatio(imageAspectRatio.ratio) | |
| .clip(RoundedCornerShape(16.dp)), | |
| .aspectRatio(currentAspectRatio.ratio) | |
| .clip(RoundedCornerShape(16.dp)) | |
| Box(modifier = modifier) { | |
| HorizontalPager( | |
| state = pagerState, | |
| contentPadding = PaddingValues(horizontal = 20.dp), | |
| pageSpacing = 10.dp, | |
| modifier = Modifier.animateContentSize(), | |
| ) { page -> | |
| val pageAspectRatio = imageAspectRatios.getOrElse(page) { ImageAspectRatio.SQUARE } | |
| Box( | |
| modifier = | |
| Modifier | |
| .fillMaxWidth() | |
| .aspectRatio(pageAspectRatio.ratio) | |
| .clip(RoundedCornerShape(16.dp)) | |
| .clickable { onFullscreenClick(page) }, | |
| ) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/components/FeedCard.kt`
around lines 315 - 329, The bug is that currentAspectRatio (from
imageAspectRatios.getOrElse(pagerState.currentPage)) is reused for every page,
causing non-active pages to render at the wrong aspect; change the
HorizontalPager item to use a page-specific ratio by calling
imageAspectRatios.getOrElse(page) inside the pager lambda for the inner Box, and
if you want the overall card container to animate by the current page height
keep a separate computed containerAspectRatio (based on pagerState.currentPage)
applied to the outer Box (with animateContentSize) while each page's inner Box
uses its own aspectRatio retrieved by page; update references to
currentAspectRatio, imageAspectRatios, pagerState, and the Box inside the
HorizontalPager accordingly.
Comment on lines
+368
to
+379
| if (page == 0 && productLink != null) { | ||
| Box( | ||
| modifier = | ||
| Modifier | ||
| .align(Alignment.TopEnd) | ||
| .padding(top = 16.dp, end = 6.dp), | ||
| contentAlignment = Alignment.TopEnd, | ||
| ) { | ||
| LinkButton( | ||
| modifier = Modifier.padding(end = 10.dp), | ||
| onClick = { onLinkClick(productLink) }, | ||
| ) |
There was a problem hiding this comment.
빈 링크도 유효한 상품 링크처럼 노출됩니다.
productLink != null만 검사하면 ""나 공백 문자열도 버튼/툴팁이 노출되고, 클릭 시 빈 URL로 WebView를 열 수 있습니다. productLink가 optional 필드인 만큼 blank도 같이 걸러두는 게 안전합니다.
🔧 제안 수정
- if (page == 0 && productLink != null) {
+ val normalizedProductLink = productLink?.takeIf { it.isNotBlank() }
+ if (page == 0 && normalizedProductLink != null) {
Box(
modifier =
Modifier
.align(Alignment.TopEnd)
.padding(top = 16.dp, end = 6.dp),
contentAlignment = Alignment.TopEnd,
) {
LinkButton(
modifier = Modifier.padding(end = 10.dp),
- onClick = { onLinkClick(productLink) },
+ onClick = { onLinkClick(normalizedProductLink) },
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/components/FeedCard.kt`
around lines 368 - 379, The current guard only checks productLink != null which
still allows empty or whitespace strings to render LinkButton and open an
invalid WebView; change the condition to filter out blank links (use
productLink.isNullOrBlank() or !productLink.isNullOrBlank()) so the LinkButton
block (LinkButton, onLinkClick usage) only runs when productLink contains a
non-blank URL (and pass the non-blank string into onLinkClick).
Comment on lines
+41
to
+44
| val arrowCenterX = size.width - arrowOffsetPx | ||
| moveTo(arrowCenterX - arrowWidthPx / 2, arrowHeightPx) | ||
| lineTo(arrowCenterX, 0f) | ||
| lineTo(arrowCenterX + arrowWidthPx / 2, arrowHeightPx) |
There was a problem hiding this comment.
화살표 중심 좌표를 경계 내로 보정해 주세요.
Line 41 계산값은 arrowOffsetFromRight나 툴팁 폭이 예상과 달라질 때 경계를 벗어나 화살표가 잘리거나 모양이 깨질 수 있습니다. 최소/최대 범위로 clamp 해두는 편이 안전합니다.
수정 예시 diff
- val arrowCenterX = size.width - arrowOffsetPx
+ val halfArrowWidth = arrowWidthPx / 2f
+ val minCenterX = halfArrowWidth
+ val maxCenterX = size.width - halfArrowWidth
+ val arrowCenterX =
+ if (minCenterX <= maxCenterX) {
+ (size.width - arrowOffsetPx).coerceIn(minCenterX, maxCenterX)
+ } else {
+ size.width / 2f
+ }
moveTo(arrowCenterX - arrowWidthPx / 2, arrowHeightPx)
lineTo(arrowCenterX, 0f)
lineTo(arrowCenterX + arrowWidthPx / 2, arrowHeightPx)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| val arrowCenterX = size.width - arrowOffsetPx | |
| moveTo(arrowCenterX - arrowWidthPx / 2, arrowHeightPx) | |
| lineTo(arrowCenterX, 0f) | |
| lineTo(arrowCenterX + arrowWidthPx / 2, arrowHeightPx) | |
| val halfArrowWidth = arrowWidthPx / 2f | |
| val minCenterX = halfArrowWidth | |
| val maxCenterX = size.width - halfArrowWidth | |
| val arrowCenterX = | |
| if (minCenterX <= maxCenterX) { | |
| (size.width - arrowOffsetPx).coerceIn(minCenterX, maxCenterX) | |
| } else { | |
| size.width / 2f | |
| } | |
| moveTo(arrowCenterX - arrowWidthPx / 2, arrowHeightPx) | |
| lineTo(arrowCenterX, 0f) | |
| lineTo(arrowCenterX + arrowWidthPx / 2, arrowHeightPx) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@core/designsystem/src/main/java/com/sseotdabwa/buyornot/core/designsystem/shape/TopArrowBubbleShape.kt`
around lines 41 - 44, The computed arrowCenterX (currently size.width -
arrowOffsetPx used before moveTo/lineTo) can fall outside the tooltip bounds
when arrowOffsetFromRight or tooltip width varies; clamp arrowCenterX to the
valid range [arrowWidthPx/2, size.width - arrowWidthPx/2] before calling
moveTo/lineTo to ensure the arrow isn't drawn outside the shape. Update the
calculation in TopArrowBubbleShape (where arrowCenterX is defined) to clamp the
value and then use that clamped arrowCenterX in the existing moveTo and lineTo
calls.
Comment on lines
+174
to
+185
| private fun handleCategoryToggled(category: FeedCategory) { | ||
| updateState { state -> | ||
| val updated = | ||
| if (category in state.selectedCategories) { | ||
| state.selectedCategories - category | ||
| } else { | ||
| state.selectedCategories + category | ||
| } | ||
| state.copy(selectedCategories = updated) | ||
| } | ||
| loadFeeds() | ||
| } |
There was a problem hiding this comment.
카테고리 토글 시 불필요한 API 호출
handleCategoryToggled에서 loadFeeds()를 호출하고 있어 카테고리 선택/해제마다 API 요청이 발생합니다. 그러나 스펙 문서(docs/superpowers/specs/2026-04-13-filter-sort-design.md 40-41행)에 따르면 카테고리 필터 변경 시 "API 재호출 없이 allFeeds 재필터링"으로 처리해야 합니다.
현재 구현은 매번 서버 호출이 발생하여 불필요한 네트워크 요청과 latency가 생깁니다.
🔧 스펙에 맞는 로컬 필터링 구현 제안
private fun handleCategoryToggled(category: FeedCategory) {
updateState { state ->
val updated =
if (category in state.selectedCategories) {
state.selectedCategories - category
} else {
state.selectedCategories + category
}
- state.copy(selectedCategories = updated)
+ state.copy(
+ selectedCategories = updated,
+ feeds = applyCategories(state.allFeeds, updated),
+ )
}
- loadFeeds()
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/home/src/main/java/com/sseotdabwa/buyornot/feature/home/ui/HomeViewModel.kt`
around lines 174 - 185, handleCategoryToggled currently calls loadFeeds causing
an unnecessary API call on every toggle; change it to only update
selectedCategories and re-filter locally from the cached feed list instead of
invoking loadFeeds. Specifically, in handleCategoryToggled (and the updateState
block that updates selectedCategories) remove the loadFeeds() call and set the
ViewModel state’s feeds field by filtering state.allFeeds (or the cached
allFeeds collection) according to the new selectedCategories so the UI updates
locally without network requests.
Comment on lines
+49
to
+60
| is UploadIntent.AddImages -> { | ||
| val existing = currentState.selectedImageUris.toSet() | ||
| val remaining = MAX_IMAGE_COUNT - currentState.selectedImageUris.size | ||
| val deduped = intent.uris.filter { it !in existing } | ||
| val toAdd = deduped.take(remaining) | ||
| val hasDuplicates = deduped.size < intent.uris.size | ||
| val hasOverflow = toAdd.size < deduped.size | ||
| updateState { it.copy(selectedImageUris = it.selectedImageUris + toAdd) } | ||
| when { | ||
| hasOverflow -> sendSideEffect(UploadSideEffect.ShowSnackbar("최대 ${MAX_IMAGE_COUNT}장까지 추가할 수 있어요")) | ||
| hasDuplicates -> sendSideEffect(UploadSideEffect.ShowSnackbar("이미 추가된 사진은 제외됐어요")) | ||
| } |
There was a problem hiding this comment.
같은 선택 배치 내부의 중복 URI는 아직 걸러지지 않습니다.
지금 로직은 기존 목록(existing)에 없는지만 보므로, 한 번의 picker 결과에 같은 URI가 두 번 들어오면 둘 다 추가됩니다. 이 경우 중복 제거 snackbar도 뜨지 않아 PR 설명의 동작과 어긋납니다.
🔧 수정 예시
is UploadIntent.AddImages -> {
val existing = currentState.selectedImageUris.toSet()
val remaining = MAX_IMAGE_COUNT - currentState.selectedImageUris.size
- val deduped = intent.uris.filter { it !in existing }
+ val incoming = intent.uris.distinct()
+ val deduped = incoming.filter { it !in existing }
val toAdd = deduped.take(remaining)
- val hasDuplicates = deduped.size < intent.uris.size
+ val hasDuplicates = incoming.size < intent.uris.size || deduped.size < incoming.size
val hasOverflow = toAdd.size < deduped.size
updateState { it.copy(selectedImageUris = it.selectedImageUris + toAdd) }
when {
hasOverflow -> sendSideEffect(UploadSideEffect.ShowSnackbar("최대 ${MAX_IMAGE_COUNT}장까지 추가할 수 있어요"))
hasDuplicates -> sendSideEffect(UploadSideEffect.ShowSnackbar("이미 추가된 사진은 제외됐어요"))
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/upload/src/main/java/com/sseotdabwa/buyornot/feature/upload/ui/UploadViewModel.kt`
around lines 49 - 60, The AddImages handler (UploadIntent.AddImages) fails to
dedupe duplicates inside the incoming batch: compute a batch-unique list first
(e.g., make intent.uris distinct/preserve order) before comparing against
currentState.selectedImageUris and MAX_IMAGE_COUNT; use that batch-unique list
instead of deduped so same-chooser duplicates are removed, then compute toAdd =
batchUnique.filter { it !in existing }. Update hasDuplicates to reflect if
batchUnique.size < intent.uris.size (or if any incoming duplicates were dropped)
and keep overflow logic based on toAdd vs batchUnique, then
updateState(selectedImageUris = ... + toAdd) and send the appropriate
UploadSideEffect.ShowSnackbar messages.
Comment on lines
+87
to
+90
| if (!LinkValidator.isValid(currentState.link)) { | ||
| sendSideEffect(UploadSideEffect.ShowSnackbar("링크 주소를 다시 확인해 주세요.")) | ||
| return | ||
| } |
There was a problem hiding this comment.
제출 시점의 title/link를 스냅샷으로 고정해 주세요.
여기서는 링크를 먼저 검증해 두고도, 실제 요청에는 업로드가 끝난 뒤의 currentState.title / currentState.link를 다시 읽습니다. 업로드 중 입력값이 바뀌면 검증한 값과 저장되는 값이 달라질 수 있습니다.
🔧 수정 예시
private fun submitFeed(context: Context) {
if (currentState.isLoading) return
- if (!LinkValidator.isValid(currentState.link)) {
+ val title = currentState.title.takeIf { it.isNotBlank() }
+ val link = currentState.link.takeIf { it.isNotBlank() }
+
+ if (!LinkValidator.isValid(link.orEmpty())) {
sendSideEffect(UploadSideEffect.ShowSnackbar("링크 주소를 다시 확인해 주세요."))
return
}
val uris = currentState.selectedImageUris
@@
feedRepository.createFeed(
category = category,
price = price,
content = content,
images = feedImages,
- title = currentState.title.takeIf { it.isNotBlank() },
- link = currentState.link.takeIf { it.isNotBlank() },
+ title = title,
+ link = link,
)
}.onSuccess {Also applies to: 120-126
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/upload/src/main/java/com/sseotdabwa/buyornot/feature/upload/ui/UploadViewModel.kt`
around lines 87 - 90, The code validates currentState.link but later reads
mutable currentState.title/currentState.link again, so snapshot the inputs at
submission time: in UploadViewModel's submit/upload handler (the method
containing LinkValidator.isValid and the sendSideEffect call) capture val
titleSnapshot = currentState.title and val linkSnapshot = currentState.link
before validating; use linkSnapshot for LinkValidator.isValid and thereafter
pass titleSnapshot and linkSnapshot into the upload request and any side-effects
instead of reading currentState again; apply the same snapshot approach to the
similar block referenced at the area corresponding to lines 120-126.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🛠 Related issue
closed #86
어떤 변경사항이 있었나요?
✅ CheckPoint
✏️ Work Description
홈 피드
FeedCard
PointerEventPass.Initial활용)업로드
GetMultipleContents→PickMultipleVisualMedia(maxItems = 3)로 OS 레벨 장수 제한내 피드 빈 화면
img_my_feed_empty이미지 추가 및BuyOrNotImgs.MyFeedEmpty등록😅 Uncompleted Tasks
📢 To Reviewers
PointerEventPass.Initial로 툴팁 dismiss를 구현한 부분 (FeedCard.kt) 확인 부탁드립니다. 자식 클릭 이벤트를 consume하지 않고 관찰만 하는 방식입니다.Summary by CodeRabbit
Release Notes