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] #57 - 비밀번호 변경 뷰 구현 #58

Merged

Conversation

lsj8706
Copy link
Member

@lsj8706 lsj8706 commented Dec 26, 2022

🌴 PR 요약

🌱 작업한 브랜치

🌱 PR Point

  • 설정 뷰의 비밀번호 변경 뷰 UI 및 서버 연결까지 구현했습니다.
  • 준호 선배의 의견대로 SettingRepository는 공유해서 사용하는 방식으로 구현했습니다!
  • SettingScene 안에 해당되는 뷰라 폴더링을 어떻게 해야 할지 몰라서 일단은 SettingScene 내부에 새롭게 PasswordSettingScene 폴더를 생성하여 작업했습니다. 논의 후 위치 변경하겠습니다..!

📌 참고 사항

  • 현재 로그인 API 연결이 되어 있지 않아 비밀번호 변경 API 헤더에 필요한 userId는 임의로 12로 넣어두었습니다.
  • 기획 문서에 따라 비번 변경이 성공하면 메인 설정 뷰로 화면 전환 + 토스트가 보여지도록 했습니다.
  • 피드백 언제나 환영합니다~!!

📸 스크린샷

|기능|스크린샷|
|비밀번호 변경 뷰| Simulator Screen Recording - iPhone 13 mini - 2022-12-26 at 17 05 45|

📮 관련 이슈

@lsj8706 lsj8706 added Feat 새로운 기능 구현 세진🤝🏻 labels Dec 26, 2022
@lsj8706 lsj8706 self-assigned this Dec 26, 2022
Copy link
Contributor

@L-j-h-c L-j-h-c left a comment

Choose a reason for hiding this comment

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

비슷한 뷰가 있는데 잘 참고하겠슴당~! 수고하셨어요!

public var viewModel: PasswordChangeViewModel!
private var cancelBag = CancelBag()

@Published public var passwordChangeSuccessed = false
Copy link
Contributor

Choose a reason for hiding this comment

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

ViewModel의 Output을 쓰지 않고 뷰컨에 값을 정의한 이유가 있을까요??

Copy link
Member Author

Choose a reason for hiding this comment

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

Toast알림이 이전 뷰인 SettingVC에서 나와야하는데 SettingVC에서 비밀번호 변경 성공 유무를 파악하려면 PasswordChangeViewModel의 Output이 필요합니다. 이걸 곧바로 가져와서 사용하는 방법이 없을 거 같아서 SettingVC에서 구독할 새로운 Publisher를 생성하여 구현했는데 저도 이 방식이 깔끔해 보이지는 않아요 ㅠㅠ
혹시 구체적으로 어떻게 개선할 수 있을까요..?

Copy link
Contributor

Choose a reason for hiding this comment

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

@lsj8706 아하 데이터 전달의 목적이였군요!
문제가 되는 부분은 현재 showToast가 VC에 addsubview 되어 vc pop 시에 같이 사라지는 것 같은데,
showToast를 window에 addsubView하는 방법으로 하면 output으로만 해결 가능할 것 같아요!

        output.passwordChangeSuccessed.sink { [weak self] isSuccess in
            guard let self = self else { return }
            self.passwordChangeSuccessed = isSuccess
            if isSuccess {
                self.navigationController?.popViewController(animated: true)
                let window = UIApplication.shared.keyWindow!
                Toast.show(message: "메시지", controller: window)
            } else {
                self.showToast(message: I18N.Setting.passwordEditFail)
            }
        }.store(in: cancelBag)

Copy link
Member Author

@lsj8706 lsj8706 Dec 28, 2022

Choose a reason for hiding this comment

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

오호 window에 addSubview를 하면 되는군요..!! 또 한번 배워갑니다!

우선 현재 showToast는 VC에만 사용이 가능한데 window는 VC가 아니라 UIView라서 UIView에서 Toast를 보여주는 showInView 함수를 생성했습니다. 그냥 show랑 너무 중복되는 부분이 많아서 수정이 필요할 거 같긴합니다.

기존의 Toast에 있는 show 함수가 파라미터로 VC를 받는 것이 아니라 UIView를 받도록 변경하고 bottomInset 로직을 수정했습니다. 이렇게 하면 UIView에 Toast를 보여줄 때에도 동일한 함수를 사용할 수 있을 거 같은데 한번 보고 피드백 주세요..!

let window = UIApplication.shared.keyWindow!는 deprecated 되었다고 해서 let window = self.view.window! 로 사용했습니다. 둘의 차이는 잘 모르겠네요ㅠ

이렇게 수정해서 올렸습니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

@lsj8706 넵 굿굿입니당~~! 저도 이런 방식을 생각했었어요! 수고하셨슴다~!

@devxsby devxsby changed the title [Fea] #57 - 비밀번호 변경 뷰 구현 [Feat] #57 - 비밀번호 변경 뷰 구현 Dec 26, 2022
Copy link
Member

@yangsubinn yangsubinn left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!
현재 폴더링대로 유지해도 괜찮을 것 같고,
관련 뷰가 많기 때문에 그냥 SettingPasswordScene을 SettingScene과 동일한 계층에 둬두 괜찮을 것 같아여

테스트가 필요하시면 제 계정 userId 사용하셔두 될 것 같습니다!

Copy link
Member

@devxsby devxsby left a comment

Choose a reason for hiding this comment

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

고생하셨어영 감사합니당 ~

@lsj8706 lsj8706 merged commit c443497 into sopt-makers:develop Dec 29, 2022
@lsj8706 lsj8706 deleted the feat/#57-비밀번호-변경-뷰-구현 branch December 29, 2022 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feat 새로운 기능 구현 size/L 세진🤝🏻
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] 비밀번호 변경 뷰 구현
4 participants