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

Feat/week 뷰모델 분리 - compose #16

Open
wants to merge 6 commits into
base: develop-compose
Choose a base branch
from

Conversation

seohee0925
Copy link
Collaborator

@seohee0925 seohee0925 commented May 24, 2024

📌 Issues

📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯

  • SignUp은 분리되어 있어서 Home만 ViewModel로 분리하였습니다
  • 명예 오비님 덕분에 MyPageFragment에서 화면이 안 보이는 에러를 수정했습니다

📷 Screenshot

💬To Reviewers

week04_compose에서 브랜치 파서 했습니다!
시험 끝나고 꼭꼭 할게요 ㅠㅠ

@seohee0925 seohee0925 added 필수 🌱 필수 과제 compose compose로 구현 labels May 24, 2024
@seohee0925 seohee0925 self-assigned this May 24, 2024
@seohee0925 seohee0925 changed the title Feat/week compose6 Feat/week 뷰모델 분리 - compose May 24, 2024
Copy link
Member

@Eonji-sw Eonji-sw left a comment

Choose a reason for hiding this comment

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

수고하셨습니당! loginViewModel 분리해주세요

@Serializable
data class ResponseFriendDto(
@SerialName("data")
val `data`: List<FriendData>,
Copy link
Member

Choose a reason for hiding this comment

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

` 의도한게 아니라면 빼는게 좋겠어요

import retrofit2.http.POST

interface AuthService {
@POST("member/join")
Copy link
Member

Choose a reason for hiding this comment

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

상수화하여 재사용성을 높이면 좋을 것 같아요

interface FriendService {
@GET("users")
fun getFriend(
@Query("page") page: Int
Copy link
Member

Choose a reason for hiding this comment

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

이 부분도 같이 상수화하면 좋을 것 같아요

interface UserService {
@GET("member/info")
fun getUser(
@Header("memberId") userId: String?
Copy link
Member

Choose a reason for hiding this comment

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

왜 nullable하게 했나요?

phone = "010-5678-1234"
)
)
val userData: MutableState<ResponseUserDto.UserData?> = mutableStateOf(null)
Copy link
Member

Choose a reason for hiding this comment

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

mutableStateOf 잘 적용하였네요! 그런데 UserData는 왜 nullable 하나욤??

@@ -0,0 +1,7 @@
package com.sopt.now.compose.ui.login

data class LoginState(
Copy link
Member

Choose a reason for hiding this comment

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

UiState에 대해서도 학습해보고 적용하면 좋을 것 같아요!

https://jminie.tistory.com/189

Comment on lines +14 to +15
private val _userData = MutableLiveData<ResponseUserDto.UserData>()
val userData: LiveData<ResponseUserDto.UserData> = _userData
Copy link
Member

Choose a reason for hiding this comment

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

이 부분도 homeViewModel 처럼 수정하면 좋을 것 같아요


Log.d("MyPageViewModel", "User data: ${_userData.value}")
} else {
Log.d("MyPageViewModel", "Failed to load user data: $userId")
Copy link
Member

Choose a reason for hiding this comment

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

여기에도 userId보다는 error message를 띄워주는게 디버깅하기 더 좋을 것 같아요!

Comment on lines +187 to +194
val signUpRequest = RequestSignUpDto(
authenticationId = userId,
password = userPassword,
nickname = userNickname,
phone = userPhone
)

val user = User(userId, userPassword, userNickname, userPhone)
val intent = Intent(context, LoginActivity::class.java)
intent.putExtra("user", user)
viewModel.signUp(signUpRequest)
Copy link
Member

Choose a reason for hiding this comment

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

viewModel.signUp(RequestSignUpDto(
                                authenticationId = userId,
                                password = userPassword,
                                nickname = userNickname,
                                phone = userPhone
                            ))

response: Response<ResponseSignUpDto>,
) {
if (response.isSuccessful) {
val data: ResponseSignUpDto? = response.body()
Copy link
Member

Choose a reason for hiding this comment

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

쓰는 부분이 있나요?

@seohee0925 seohee0925 linked an issue May 26, 2024 that may be closed by this pull request
2 tasks
Copy link

@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.

제가 넘 늦었네요 죄송해요😭😭😭 근데 정말이지 짱이다,,, 뷰모델 분리 넘 잘하셨네요!! 최고최고👍🏻

items(friendList) {
FriendProfileItem(it)
items(friendList) { friend ->
FriendProfileItem(friend)

Choose a reason for hiding this comment

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

it으로 안하고 friend로 네이밍 해준거 넘 좋네요!!

class SignUpViewModel : ViewModel() {
private val authService by lazy { ServicePool.authService }

val liveData = MutableLiveData<SignUpState>()

Choose a reason for hiding this comment

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

이거는 get() 써서 _liveData 이렇게 처리 안해줘도 괜찮나요???(궁금해서 여쭤보는거에용ㅎㅎ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compose compose로 구현 필수 🌱 필수 과제
Projects
None yet
Development

Successfully merging this pull request may close these issues.

week06 view model과 ui 로직 분리 - compose
3 participants