Skip to content
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

Feature/boolti 298 공연 상세 UI 변경 (출연진 탭 추가) #299

Merged
merged 7 commits into from
Oct 1, 2024

Conversation

mangbaam
Copy link
Member

@mangbaam mangbaam commented Oct 1, 2024

Issue

작업 내용

  • 공연 상세 UI 변경
    • 출연진 탭 추가
  • 공연 상세 ColumnLazyColumn 리팩터링
  • 유저 섬네일 컴포저블 컴포넌트화 (UserThumbnail)
    • 프로필, 마이 탭, 프로필 수정에서 UserThumbnail 로 대체
    • 공연 상세 출연진에서도 사용
image image

@mangbaam mangbaam requested a review from HamBP October 1, 2024 16:25
Copy link

github-actions bot commented Oct 1, 2024

Test Results

 9 files   9 suites   0s ⏱️
 8 tests  8 ✅ 0 💤 0 ❌
12 runs  12 ✅ 0 💤 0 ❌

Results for commit aa5734d.

♻️ This comment has been updated with latest results.

Copy link
Member

@HamBP HamBP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines 15 to 16
val createdAt: String = LocalDateTime.MIN.format(DateTimeFormatter.ISO_LOCAL_DATE_TIME),
val modifiedAt: String? = null,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 내 예상이 맞다면 DB 관리 차원에서 항상 들어가는 column이 같이 내려온 거 같은데, UI에서 안 쓰이면 안 받아도 될 듯!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

처리 완료

}

fun selectTab(index: Int) {
_uiState.update { it.copy(selectedTab = index.coerceIn(0 .. 1)) }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안정성 👍

Column(
modifier = Modifier
.fillMaxWidth()
.padding(vertical = 117.dp),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

개인적으로는 패딩보다는 높이를 290으로 지정하고 가운데 정렬할 거 같긴 한데, 참고만 해주세여~

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

처리 완료

Comment on lines +544 to +547
/**
* 중첩 Lazy 레이아웃 처리를 위해 높이 고정 필요
*/
val gridHeight = memberHeight * rows + spacedBySize * (rows - 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[QQ] 이걸로 중첩 스크롤 막은 건가?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lazy 안에 Lazy가 있으면 높이를 계산할 수 없어서 Exception 발생해서 크래시 터져
그래서 높이를 지정해줘야 해

@mangbaam mangbaam merged commit c006cfe into develop Oct 1, 2024
2 checks passed
@mangbaam mangbaam deleted the feature/Boolti-298 branch October 1, 2024 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

출연자 목록 표시
2 participants