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

테마 마켓 추가 #383

Merged
merged 37 commits into from
Feb 22, 2025
Merged

Conversation

eastshine2741
Copy link
Collaborator

@eastshine2741 eastshine2741 commented Dec 15, 2024

#382 의 변경사항 위에서 작업했어요

  • 테마설정 페이지에서 담은테마 보여주기
  • 테마마켓 진입점, 화면 생성
  • 테마마켓용 웹뷰 추가
  • LoadState.InitialLoading 삭제

@eastshine2741 eastshine2741 requested a review from a team as a code owner December 15, 2024 07:16
factory = {
webView
},
modifier = modifier.clipToBounds(), // Compose에서 WebView 사용 시, WebView가 잠깐 동안 다른 Composable을 가리는 WebView 버그 대응(https://issuetracker.google.com/issues/174233728?pli=1#comment5)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

안드 웹뷰가 그려질 때 잠깐 동안 다른 Composable을 가리는 버그가 있음
Modifier.clipToBounds()를 적용해서 웹뷰가 자신의 bound을 넘어서 그려지지 않도록 강제

https://issuetracker.google.com/issues/174233728?pli=1#comment5

Base automatically changed from eastshine2741/refactor-custom-theme to develop January 27, 2025 13:03
Copy link
Collaborator

@plgafhd plgafhd left a comment

Choose a reason for hiding this comment

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

오옹... 신기하다

Comment on lines +66 to +71
@Composable
fun MarketCustomThemeMoreActionBottomSheet(
onClickDetail: () -> Unit,
onClickDelete: () -> Unit,
modifier: Modifier = Modifier,
) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

흐음 혹시 이 MarketCustomThemeMoreActionBottomSheetMyCustomThemeMoreActionBottomSheet 잘 합치는건 별로인가..?
굳이 무리한 합치기이면 필요 없긴 한데 약간 중복 느낌도 있어서
+) 으음 전체적으로 따로따로 있구나 굳이 합칠필요 없는것 같기도

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

마자 공용 함수로 합치면 MarketCustomTheme에는 필요없는 콜백을 MarketCustomTheme이 받게 되는 게 싫어서 분리했어


@Composable
fun ThemeMarketScreen(
accessToken: StateFlow<String>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

route 에서 collect 하고 값으로 넘기는 거랑 어떤 차이가 있지? (진짜모름)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ThemeMarketWebViewContainer가 StateFlow로 받고 있어
ThemeMarketRoute에서 collect하고 값으로 넘기기엔 결국 StateFlow를 다시 만들어야 하고
ThemeMarketWebViewContainer가 값으로 받도록 바꾸기엔 token이 바뀌면 객체를 새로 만들어야 하고
고민하다가 하던대로 해버렸습니다

Copy link
Collaborator

@JuTaK97 JuTaK97 left a comment

Choose a reason for hiding this comment

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

나는 LGTM~

@JuTaK97
Copy link
Collaborator

JuTaK97 commented Feb 9, 2025

마무리해줘 우애애앵

Copy link
Collaborator

@JuTaK97 JuTaK97 left a comment

Choose a reason for hiding this comment

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

고생하셨수

@eastshine2741 eastshine2741 enabled auto-merge (squash) February 22, 2025 08:01
@eastshine2741 eastshine2741 enabled auto-merge (squash) February 22, 2025 08:05
@eastshine2741 eastshine2741 merged commit 61c596b into develop Feb 22, 2025
3 checks passed
@eastshine2741 eastshine2741 deleted the eastshine2741/add-custom-theme-market branch February 22, 2025 08:09
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.

3 participants