[Session] 세션 상세정보 화면 디자인 변경 #239
Merged
Conversation
This file contains 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
laco-dev
reviewed
Aug 27, 2023
feature/session/src/main/java/com/droidknights/app2023/feature/session/SessionDetailScreen.kt
Outdated
Show resolved
Hide resolved
feature/session/src/main/java/com/droidknights/app2023/feature/session/SessionDetailScreen.kt
Outdated
Show resolved
Hide resolved
feature/session/src/main/java/com/droidknights/app2023/feature/session/SessionDetailScreen.kt
Outdated
Show resolved
Hide resolved
| speakers.forEach { speaker -> | ||
| NetworkImage( | ||
| imageUrl = speaker.imageUrl, |
There was a problem hiding this comment.
스펙상 스피커가 여러명인것 같은데 실제로는 한 명만 존재하고 디자인에 고려되어있지 않아서
여러명인 경우의 디자인을 고려하지 않고 작성하였습니다!
명확하게 스피커 한명에 대한 코드로 만들어보면 어떨까요?
*이미 두명 이상을 고려하지 않고 있으니 코드로 유지할 필요가 없음
There was a problem hiding this comment.
혹시 스피커를 한명에 대한 코드로 만들자는 내용이 Session 모델부터 개선하는것을 말씀주신 것이었을까요??
개인적으로는 이 Issue와는 별개의 태스크라고 생각되어 SessionDetail에서 first로 접근하도록 변경하였습니다!
이 PR에서 처리되는것을 의도하신것이라면 말씀주시면 감사하겠습니다 !!
953288a
There was a problem hiding this comment.
아하 네 말씀주신 부분을 의도한 것이 맞는데, 별도 이슈로 판단해도 될 것 같습니다. 🙏🏻
|
TagChips를 List 타입으로 받고 있었네요 😅 처음 리뷰해주신 내용이랑 달라져서 직접 언급드려봅니다! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue
Overview (Required)
-> 추후 PR에서 세션 목록화면의 SessionCard에 북마크 칩이 들어가는것을 고려하여,
분기를 태우거나 Composable 함수를 열어두는것보다 제거하는것이 더 낫다고 판단하였는데
잘못 판단했다 생각되신다면 의견 주시면 감사하겠습니다!
여러명인 경우의 디자인을 고려하지 않고 작성하였습니다!
Links
Screenshot