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

ReviewFragment & MenuDetailFragment 리팩터링 #52

Open
wants to merge 70 commits into
base: develop
Choose a base branch
from

Conversation

wjshim2003
Copy link
Collaborator

No description provided.

@wjshim2003
Copy link
Collaborator Author

@sanggggg ReviewFragment랑 ReviewPhotoFragment의 차이가 사진을 보이느냐 마냐 말곤 없길래 컴포즈로 옮길 때에는 같은 컴포저블 함수를 사용하되 가져오는 API와 사진 표시 플래그에만 차이를 두려고 합니다. 혹시 원래 분리해서 관리하던 이유나 합쳤을 때 예상되는 문제점이 있을까요?

@sanggggg
Copy link
Collaborator

@sanggggg ReviewFragment랑 ReviewPhotoFragment의 차이가 사진을 보이느냐 마냐 말곤 없길래 컴포즈로 옮길 때에는 같은 컴포저블 함수를 사용하되 가져오는 API와 사진 표시 플래그에만 차이를 두려고 합니다. 혹시 원래 분리해서 관리하던 이유나 합쳤을 때 예상되는 문제점이 있을까요?

제가 작업한 내용은 아니어서 정확하게는 모르겠지만, 훑어보니 그렇게 가져간다고 해서 문제가 될 부분은 없어보입니다.
오히려 UI 랑 분리되면 더 좋아보입니다 👍

Copy link
Collaborator Author

@wjshim2003 wjshim2003 left a comment

Choose a reason for hiding this comment

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

이럴 수가... SikshaTheme 적용을 안 해 놨었어

Comment on lines 71 to 73
private val _leaveLeaveReviewState = MutableLiveData<LeaveReviewState>(LeaveReviewState.WAITING)
val leaveLeaveReviewState: LiveData<LeaveReviewState>
get() = _leaveLeaveReviewState
Copy link
Collaborator

Choose a reason for hiding this comment

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

leaveLeave 뭐야 오타?

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
fun MenuRatingStars(
rating: Float,
modifier: Modifier = Modifier.width(100.dp).height(18.dp),
Copy link
Collaborator

Choose a reason for hiding this comment

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

사실 modifier 파라미터가 default argument 가지는 게 권장되지 않긴 해
https://stackoverflow.com/questions/73415055/why-was-i-warned-optional-modifier-parameter-should-have-a-default-value-of-mod
이 링크에 있는대로 size: Dp를 다른 파라미터로 받는 게 좋을듯?

@eastshine2741
Copy link
Collaborator

Screen, Route 분리하고
Preview 쪼금 추가하고(나중에 더 세세하게 추가하기)
develop 땡겨오고 컨플릭트 수정하고
그외 네이밍수정같은거 조금함

커뮤니티 배포랑 같이 나가면 정신없을거같으니 일단 머지하고 배포한번할까?

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