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

[PC-300] 약관 디테일 화면 구현 #27

Merged
merged 12 commits into from
Jan 8, 2025
Merged

Conversation

tgyuuAn
Copy link
Member

@tgyuuAn tgyuuAn commented Jan 8, 2025

PC-300

1. ⭐️ 변경된 내용

  • 약관 디테일 화면을 구현하였습니다.
  • 디자인 시스템에서 TopBar의 높이가 60 고정인 것을 확인하여 높이를 60을 고정으로 주었습니다.
  • 약관 상세 페이지를 웹뷰로 뛰우기로 하여서 PieceWebView 컴포넌트를 만들었습니다. 해당 컴포넌트는 Setting 모듈에서도 사용할 예정이라 designSystem으로 올렸습니다.

2. 🖼️ 스크린샷(선택)

2025-01-08.1.03.01.mov

3. 📌 이 부분은 꼭 봐주세요!

제가 웹뷰를 잘 모르는데, 혹시 웹뷰 페이지가 로딩될 때 까지 상단 탑바까지 랜더링이 안되는데 이유를 아시나요 ...?

여러가지 삽질도 해보고 gpt한테 물어보기도 했는데 잘 모르겠네요 ㅠㅠㅠ

웹뷰 페이지가 랜딩이 다 되어야지만 탑바가 같이 랜더링 되어요 ㅠㅜㅠㅠㅠ

@tgyuuAn tgyuuAn added UI/UX 🎨 디자인 시스템, 디자인 리소스 관련 코드 🎨 기능 ⚒️ 새로운 기능 구현 ⚒️ 리뷰 원해요🔥 피어의 리뷰를 기다리는 ing.. 🔥 ㅌㄱ태규 ☀️ 훗날 크게될 ENFP 남성, tgyuuAn labels Jan 8, 2025
@tgyuuAn tgyuuAn requested a review from sksowk156 January 8, 2025 04:17
@tgyuuAn tgyuuAn self-assigned this Jan 8, 2025
@tgyuuAn tgyuuAn changed the title Feature/tgyuu/pc 300 [PC-300] 약관 디테일 화면 구현 Jan 8, 2025
Copy link
Contributor

@sksowk156 sksowk156 left a comment

Choose a reason for hiding this comment

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

허허 웹뷰는 저도 compose에선 안써봐서 잘 모르는겠는데... 흠... 혹시 웹뷰 배경이 topbar를 덮는건 아니겠죠?? 아니면 메인 스레드가 막혀서 그런건가?? topbar나 웹뷰 배경색 지정해서 덮는건지 아닌지 확인해보면 좋을 것 같아요...!!! 뭐 이것도 해보셨을 거 같긴 하지만... 아니면 흠.... webivewclient 메서드를 오버라이드 해서 loading 상태 추적해보면 좋을 것 같아요..!! 허허

import com.puzzle.navigation.NavigationEvent

@Composable
internal fun RegistrationRoute(
viewModel: RegistrationViewModel = mavericksViewModel()
) {
val state by viewModel.collectAsState()
val (selectedTerm, setSelectedTerm) = remember { mutableStateOf<Term?>(null) }
Copy link
Contributor

Choose a reason for hiding this comment

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

    var selectedTerm by remember { mutableStateOf<Term?>(null) }

selectedTerm을 위임이 아니라 분해 선언을 하신 이유가 궁금합니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

    var selectedTerm by remember { mutableStateOf<Term?>(null) }

selectedTerm을 위임이 아니라 분해 선언을 하신 이유가 궁금합니다!

selectedTerm 프로퍼티로 위임해서 사용하고 있었는데,

약관 상세에서 다시 이전화면으로 돌아갈 때

selectedTerm = null

이라는 코드가 약간 어색하게(?) 느껴져서 setSelectedTerm() 이라는 메서드를 사용하고 싶어서 분해하였습니다 ㅎㅎ,,


이 부분 코드 일관성을 위해서 그냥 위임하는게 나을까요 ?!

Copy link
Contributor

Choose a reason for hiding this comment

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

앗 아뇨 충분히 좋은 접근이라고 생각합니다....!! 일관성도 가독성을 위한 것이라 생각하기 때문에, setSelectedTerm으로 분리해서 쓰신 이유가 가독성이라면 굳이 일관성을 위해 다시 수정할 필요는 없을 것 같아요!

@sksowk156 sksowk156 added 머지 해도될듯염🌟 현재 코드를 기존 코드에 합쳐도 될 것 같다라고 판단..! 🌟 and removed 리뷰 원해요🔥 피어의 리뷰를 기다리는 ing.. 🔥 labels Jan 8, 2025
@tgyuuAn
Copy link
Member Author

tgyuuAn commented Jan 8, 2025

허허 웹뷰는 저도 compose에선 안써봐서 잘 모르는겠는데... 흠... 혹시 웹뷰 배경이 topbar를 덮는건 아니겠죠?? 아니면 메인 스레드가 막혀서 그런건가?? topbar나 웹뷰 배경색 지정해서 덮는건지 아닌지 확인해보면 좋을 것 같아요...!!! 뭐 이것도 해보셨을 거 같긴 하지만... 아니면 흠.... webivewclient 메서드를 오버라이드 해서 loading 상태 추적해보면 좋을 것 같아요..!! 허허

LayoutInspector랑 메서드 오버라이딩해서 디깅 더 해 보겠습니다..!

@tgyuuAn
Copy link
Member Author

tgyuuAn commented Jan 8, 2025

++

#25 내용 추가적으로 반영하였습니다.

@tgyuuAn tgyuuAn force-pushed the feature/tgyuu/PC-300 branch from bfd1f15 to b4e3fb0 Compare January 8, 2025 16:16
@tgyuuAn tgyuuAn merged commit abeb37a into develop Jan 8, 2025
1 check passed
@tgyuuAn tgyuuAn deleted the feature/tgyuu/PC-300 branch January 8, 2025 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI/UX 🎨 디자인 시스템, 디자인 리소스 관련 코드 🎨 ㅌㄱ태규 ☀️ 훗날 크게될 ENFP 남성, tgyuuAn 기능 ⚒️ 새로운 기능 구현 ⚒️ 머지 해도될듯염🌟 현재 코드를 기존 코드에 합쳐도 될 것 같다라고 판단..! 🌟
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants