-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/#215 session detail UI #216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…on) and wire into app
…ons, map placeholder, notice list) without logic/map
Walkthrough새로운 세션(feature: session) 모듈을 추가하고 네비게이션 경로와 화면(UI)을 구현했다. 앱 모듈에 의존성을 연결하고 settings에 서브프로젝트를 등록했다. 디자인시스템에 GradientTop 컴포저블을 추가하고 기존 GradientBottom 프리뷰 가시성을 private로 변경했다. 문서 AGENTS.md를 신규 추가했다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as 사용자
participant App as App(navHost)
participant Nav as NavController
participant Route as SessionRoute
participant VM as SessionViewModel
participant Store as MviIntentStore
participant UI as SessionScreen
User->>App: 세션 상세 진입 트리거
App->>Nav: navigateToSession()
Nav->>Route: composable(SessionRoute)
Route->>VM: hiltViewModel()
Note over VM,Store: 초기 상태 생성(initialState=SessionState())
Route->>Store: dispatch(EnterSessionScreen)
Store->>VM: onIntent(EnterSessionScreen)
VM->>Store: reduce { isLoading=false }
Store-->>UI: state 전달
UI-->>User: 세션 상세 화면 렌더링
User->>UI: 당겨서 새로고침(예시)
UI->>Store: dispatch(Refresh)
Store->>VM: onIntent(Refresh)
VM->>Store: reduce { isLoading=true }
VM->>Store: reduce { isLoading=false }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/session/consumer-rules.pro (1)
1-2: consumer-rules.pro가 비어 있습니다 — 적용 여부 확인 필요
라이브러리 모듈의 consumer ProGuard 파일이 실제 패키징에 포함되는지 확인해 주세요(플러그인에서 자동 지정하지 않는다면 명시 필요).적용이 필요하다면 build.gradle.kts에 다음을 추가하세요:
android { - setNamespace("feature.session") + setNamespace("feature.session") + defaultConfig { + consumerProguardFiles("consumer-rules.pro") + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
AGENTS.md(1 hunks)app/build.gradle.kts(1 hunks)core/designsystem/src/main/java/com/yapp/core/designsystem/component/gradient/GradientBottom.kt(1 hunks)core/designsystem/src/main/java/com/yapp/core/designsystem/component/gradient/GradientTop.kt(1 hunks)feature/session/.gitignore(1 hunks)feature/session/build.gradle.kts(1 hunks)feature/session/consumer-rules.pro(1 hunks)feature/session/proguard-rules.pro(1 hunks)feature/session/src/main/AndroidManifest.xml(1 hunks)feature/session/src/main/java/com/yapp/feature/session/SessionContract.kt(1 hunks)feature/session/src/main/java/com/yapp/feature/session/SessionScreen.kt(1 hunks)feature/session/src/main/java/com/yapp/feature/session/SessionViewModel.kt(1 hunks)feature/session/src/main/java/com/yapp/feature/session/navigation/SessionNavigation.kt(1 hunks)settings.gradle.kts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-01-08T06:56:07.931Z
Learnt from: jinukeu
PR: YAPP-admin/yappu-world-android#5
File: core/domain/build.gradle.kts:1-4
Timestamp: 2025-01-08T06:56:07.931Z
Learning: In this project's architecture, the domain module is designed as an Android library module, not a pure Kotlin library, following Google's recommended architecture guidelines.
Applied to files:
AGENTS.md
📚 Learning: 2025-01-09T07:44:48.036Z
Learnt from: ashwon12
PR: YAPP-admin/yappu-world-android#5
File: feature/signup/build.gradle.kts:1-47
Timestamp: 2025-01-09T07:44:48.036Z
Learning: The project follows a modular architecture with two types of modules: Core (model, data-api, designsystem, ui, data, domain) and Feature (home, notice, signup, login). Core modules provide shared functionality while Feature modules implement specific features.
Applied to files:
AGENTS.md
🪛 markdownlint-cli2 (0.17.2)
AGENTS.md
3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
4-4: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
9-9: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
10-10: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
18-18: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
23-23: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
24-24: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
28-28: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
29-29: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
33-33: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
34-34: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
🔇 Additional comments (10)
app/build.gradle.kts (1)
76-76: 세션 모듈 의존성 추가 OK. 상위 NavGraph 연결 여부만 확인해 주세요.
앱 NavGraph에 sessionNavGraph/Route가 실제로 포함되어 화면 진입이 가능한지 점검 부탁드립니다.feature/session/.gitignore (1)
1-1: 모듈 빌드 산출물 제외 설정 적절합니다.
별도 조치 없음.settings.gradle.kts (1)
48-50: 서브프로젝트 등록 LGTM.
동일한 패턴으로 feature 모듈을 관리하고 있어 일관성 좋습니다.feature/session/build.gradle.kts (1)
1-17: yapp.android.feature 플러그인이 core:ui·core:designsystem 의존성과 Compose BOM, material3, navigation-compose 등 Compose 관련 의존성을 자동으로 설정합니다. 별도 dependencies 선언을 제거하세요.Likely an incorrect or invalid review comment.
core/designsystem/src/main/java/com/yapp/core/designsystem/component/gradient/GradientBottom.kt (1)
42-42: Preview를 private으로 내린 변경 좋습니다.
외부 API 표면을 오염시키지 않아 모듈 공개 범위가 명확해집니다.feature/session/src/main/AndroidManifest.xml (1)
1-4: 라이브러리 모듈용 최소 매니페스트 적절
namespace는 Gradle에서 지정되므로 package 생략 OK. 추후 컴포넌트 추가 시 exported/permission 설정만 유의해 주세요.core/designsystem/src/main/java/com/yapp/core/designsystem/component/gradient/GradientTop.kt (1)
40-53: 프리뷰 구성은 적절합니다테마 적용과 사이즈 지정이 명확합니다.
feature/session/src/main/java/com/yapp/feature/session/navigation/SessionNavigation.kt (1)
7-12: 이름 충돌로 컴파일 오류 발생:data object SessionRoute가 Composable 함수를 가림
동일한 이름(SessionRoute)의 data object가 ComposableSessionRoute()를 섀도잉하여 컴파일 오류가 발생합니다. data object명을 변경하세요.-@Serializable -data object SessionRoute +@Serializable +data object SessionDestination @@ -fun NavController.navigateToSession(navOptions: NavOptions? = null) { - navigate(SessionRoute, navOptions) +fun NavController.navigateToSession(navOptions: NavOptions? = null) { + navigate(SessionDestination, navOptions) } @@ -fun NavGraphBuilder.sessionNavGraph() { - composable<SessionRoute> { - SessionRoute() +fun NavGraphBuilder.sessionNavGraph() { + composable<SessionDestination> { + SessionRoute() } }프로젝트에서 사용 중인 navigation-compose 버전이 typed destination API(
composable<Dest>,navigate(Dest))를 지원하는지 확인해주세요.feature/session/src/main/java/com/yapp/feature/session/SessionScreen.kt (2)
66-69: state 파라미터 미사용UI-only 1차 PR이라면 괜찮지만, 이후 로딩/데이터 바인딩에 사용할 계획을 확인해주세요.
71-73: 스크롤 판단 로직의 라이브러리 의존성 축소
ScrollState.canScrollBackward가용성은 버전에 따라 다릅니다. 값 비교로 대체하면 안정적입니다.- val showGradientBottom by remember { - derivedStateOf { scrollState.canScrollBackward } - } + val showGradientBottom by remember { + derivedStateOf { scrollState.value > 0 } + }
|
|
||
| ## Project Structure & Module Organization | ||
| - Multi‑module Android project using Gradle (Kotlin DSL). | ||
| - Modules: `:app` (entry), `:core:{model,data-api,designsystem,ui,data,domain,common-android}`, `:feature:{home,notice,signup,login,schedule,profile,history,setting}`, `:detekt` (custom rules). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
문서 최신화: 신규 모듈(feature:session) 누락
모듈 목록에 session이 없습니다. PR 내용과 불일치합니다.
-- Modules: `:app` (entry), `:core:{model,data-api,designsystem,ui,data,domain,common-android}`, `:feature:{home,notice,signup,login,schedule,profile,history,setting}`, `:detekt` (custom rules).
+- Modules: `:app` (entry), `:core:{model,data-api,designsystem,ui,data,domain,common-android}`, `:feature:{home,notice,signup,login,schedule,profile,history,setting,session}`, `:detekt` (custom rules).📝 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.
| - Modules: `:app` (entry), `:core:{model,data-api,designsystem,ui,data,domain,common-android}`, `:feature:{home,notice,signup,login,schedule,profile,history,setting}`, `:detekt` (custom rules). | |
| Modules: `:app` (entry), `:core:{model,data-api,designsystem,ui,data,domain,common-android}`, `:feature:{home,notice,signup,login,schedule,profile,history,setting,session}`, `:detekt` (custom rules). |
🤖 Prompt for AI Agents
In AGENTS.md around line 5, the modules list is missing the newly added feature
module `session`; update the Modules line to include `:feature:session`
alongside the other feature modules so the document matches the PR changes
(e.g., add `session` into the `:feature:{...}` group).
core/designsystem/src/main/java/com/yapp/core/designsystem/component/gradient/GradientTop.kt
Show resolved
Hide resolved
core/designsystem/src/main/java/com/yapp/core/designsystem/component/gradient/GradientTop.kt
Show resolved
Hide resolved
feature/session/src/main/java/com/yapp/feature/session/SessionContract.kt
Show resolved
Hide resolved
feature/session/src/main/java/com/yapp/feature/session/SessionViewModel.kt
Show resolved
Hide resolved
TaeseongYun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
codex 는 처음들어봤는데 gpt 도 많이 생겼군요
궁금한것만 코멘트 남겼고 나머지는 이상없어보입니다 !
| fun GradientTop( | ||
| modifier: Modifier, | ||
| color: Color, | ||
| ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q) 위 리뷰처럼 modifier 에 기본값 세팅 뺀거는 의도하신건가요 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 넵!
너비랑 높이 값을 Modifier를 통해서 필수로 지정하게끔 만드려고 했어용
DongChyeon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다
💡 Issue
🌱 Key changes
✅ To Reviewers
📸 스크린샷
Summary by CodeRabbit
신기능
문서
작업(Chores)
리팩터링