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

[UI/#218] 온보딩 / UI 수정 #219

Merged
merged 21 commits into from
Sep 7, 2024
Merged

[UI/#218] 온보딩 / UI 수정 #219

merged 21 commits into from
Sep 7, 2024

Conversation

leeeyubin
Copy link
Member

⛳️ Work Description

  • 필터링 설정 버튼 수정
  • 데이트 피커 수정
  • 파일 위치 이동 및 로직 수정
  • 데이터 관련 파일 만들기

📸 Screenshot

Screen_Recording_20240904_073123_.terning.mp4

📢 To Reviewers

  • 이번에 제 피알에서도 Grey375이 쓰여서 일단 넣어놨어요! 아린 언니 머지하면 지울게욤
  • 데이트 피커 관련해서 전면 수정했어요! (기존 거는 앱이 터지는 경우가 너무 많았음)
  • 이게 효빈언니랑 같이 써야 하는 거라 core 모듈에서 작성한 후, feature 모듈에서 참조하려고 했는데 rememberSnapFlingBehavior()에서 자꾸 에러가 나더라구요. 그래서 일단은 feautre 모듈 안에 넣어놨습니당,, 뭐가 문젠지 더 알아볼게요!
  • 그리구 저번에 회의 때 말했던 캘린더 확장함수 빼놨는데 효빈언니 확인 한번만 해줘요!!

@leeeyubin leeeyubin added UI 💐 UI 작업 유빈💙 유빈 labels Sep 3, 2024
@leeeyubin leeeyubin added this to the 2차 스프린트 작업 milestone Sep 3, 2024
@leeeyubin leeeyubin self-assigned this Sep 3, 2024
Copy link
Member

@boiledEgg-s boiledEgg-s left a comment

Choose a reason for hiding this comment

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

너무 고생하셨습니당~ 데이트피커 수정된거 편안하네요!!

@@ -87,7 +82,7 @@ fun FilteringButton(
color = borderColor
),
shape = RoundedCornerShape(cornerRadius),
onClick = { onButtonClick() }
onClick = onButtonClick
Copy link
Member

Choose a reason for hiding this comment

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

편-안

Comment on lines 21 to 27
fun updateButtonValidation() {
_state.value = _state.value.copy(isButtonValid = true)
}

fun updateGrade(grade: Int) {
_state.value = _state.value.copy(grade = grade)
}
Copy link
Member

Choose a reason for hiding this comment

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

최근에 든 의문인데, 예를 들어 어떤 버튼을 누르면 두 함수를 모두 호출하는 상황이 존재한다면 이렇게 나눠서 관리하는게 좋을까요 아니면 용도에 맞게 합쳐서 관리하는게 좋을까요..?
뭔가 이런 사소한 부분 때문에 리컴포지션이 한번이라도 더 발생할 수 있다는 생각이 드는데 맞는지는 모르겠네요..ㅎㅎ 어렵다

Copy link
Member

Choose a reason for hiding this comment

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

저도 관련해서 고민중이었는데 추후에 개별적으로 사용될 가능성을 고려해서 나눠놓는게 좋을까 싶다가도 확실하게 항상 같이 호출하는 상황이라면 합쳐서 관리하는게 더 효율적일 것 같기도 하고... 진짜 어렵다

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 31 to 32
onNextClick = { grade -> navHostController.navigateFilteringTwo(grade) },
navigateUp = { navHostController.navigateUp() }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
onNextClick = { grade -> navHostController.navigateFilteringTwo(grade) },
navigateUp = { navHostController.navigateUp() }
onNextClick = navHostController::navigateFilteringTwo,
navigateUp = navHostController::navigateUp

이 친구들도 요렇게 바꿀 수 있답니다!

Comment on lines 78 to 79
onYearChosen = { viewModel.updateStartYear(it) },
onMonthChosen = { viewModel.updateStartMonth(it) }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
onYearChosen = { viewModel.updateStartYear(it) },
onMonthChosen = { viewModel.updateStartMonth(it) }
onYearChosen = viewModel::updateStartYear,
onMonthChosen = viewModel::updateStartMonth

호출하는 함수가 하나이고, 인자의 순서가 맞다면 얘네도 "::"로 호출이 가능합니닷

Comment on lines 23 to 25
fun updateButtonValidation() {
_state.value = _state.value.copy(isButtonValid = true)
}
Copy link
Member

Choose a reason for hiding this comment

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

상태를 업데이트를 할 때 MutableStateFlow.update()를 사용하면 멀티 스레드 환경에서 여러 개의 스레드가 동시에 같은 변수를 초기화하는 것을 막아주는 원자성을 보장한대요!
만약 viewModelScope와 같은 코루틴들이 얽힌 경우엔 update를 사용하는 것이 안전할 것 같습니다~

Copy link
Member

Choose a reason for hiding this comment

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

데이트피커 까리해졌네요ㄷㄷ

Copy link
Member

@Hyobeen-Park Hyobeen-Park left a comment

Choose a reason for hiding this comment

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

데이트 피커 수정한거 진짜 짱이에요!! 확장함수도 확인했습니다요~! 수고하셨습니다😊😊

Copy link
Member

Choose a reason for hiding this comment

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

확인했습니당!! 최고최고💚

Copy link
Member

Choose a reason for hiding this comment

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

상수로 저장한거 좋네욤!!

Comment on lines 21 to 27
fun updateButtonValidation() {
_state.value = _state.value.copy(isButtonValid = true)
}

fun updateGrade(grade: Int) {
_state.value = _state.value.copy(grade = grade)
}
Copy link
Member

Choose a reason for hiding this comment

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

저도 관련해서 고민중이었는데 추후에 개별적으로 사용될 가능성을 고려해서 나눠놓는게 좋을까 싶다가도 확실하게 항상 같이 호출하는 상황이라면 합쳐서 관리하는게 더 효율적일 것 같기도 하고... 진짜 어렵다

@leeeyubin leeeyubin merged commit b862c0e into develop Sep 7, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI 💐 UI 작업 유빈💙 유빈
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[UI] 온보딩 / UI 수정
3 participants