[Design&Network] #233 - MyPageProfileVisibilityView UI 및 API 연결 #235
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ena-isme
changed the title
hyowon612
approved these changes
Sep 28, 2024
| }) | ||
| .disposed(by: disposeBag) | ||
| } | ||
|
|
| .disposed(by: disposeBag) | ||
|
|
||
| self.isPublic = self.initStatus | ||
| print(self.isPublic, "💖") |
Guryss
approved these changes
Sep 28, 2024
Naknakk
requested changes
Sep 30, 2024
| @@ -14,7 +14,7 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate { | |||
| func scene(_ scene: UIScene, willConnectTo session: UISceneSession, options connectionOptions: UIScene.ConnectionOptions) { | |||
| guard let windowScene = (scene as? UIWindowScene) else { return } | |||
|
|
|||
| let navigationController = UINavigationController(rootViewController: WSSTabBarController()) | |||
| let navigationController = UINavigationController(rootViewController: MyPageSettingViewController()) | |||
There was a problem hiding this comment.
W3
메인 당겨와서 브랜치 최신화 할 때 까먹지말기~~
| } | ||
|
|
||
| profilePrivateLabel.snp.makeConstraints { | ||
| $0.leading.equalToSuperview().inset(30) |
Comment on lines
+98
to
+101
| completeButton.snp.makeConstraints { | ||
| $0.width.equalTo(48) | ||
| $0.height.equalTo(42) | ||
| } |
There was a problem hiding this comment.
W1
이 버튼도 터치영역 잡아주기 보다는 없이 가도 좋을 것 같아요.
피그마에 비해 완료 버튼이 안쪽으로 밀려들어가 있는게 보여서요! 아래 체크 동그라미가 완 글자 밑에 있어야 하는데 밀려서 완료 글자 중간에 위치하게 된 것 같습니당.
Comment on lines
+63
to
+76
| input.completeButtonDidTap | ||
| .subscribe(with: self, onNext: { owner, _ in | ||
| guard owner.initStatus != owner.isPublic else { return } | ||
|
|
||
| owner.patchUserProfileVisibility(isProfilePublic: owner.isPublic) | ||
| .subscribe(onNext: { | ||
| output.popViewControllerAction.accept(true) | ||
| //TODO: - toastMessage | ||
| }, onError: { error in | ||
| print(error) | ||
| }) | ||
| .disposed(by: owner.disposeBag) | ||
| }) | ||
| .disposed(by: disposeBag) |
There was a problem hiding this comment.
W1
subscribe 안에 또 subscribe가 있는게 좋지 않은 것 같습니다. 그럼 해당 이벤트가 일어날 때마다 매번 내부에서는 새로운 구독이 추가되는건데, 메모리 측면에서 좋지 않은 것 같아요. 의도치 않은 동작을 만들 수도 있을 것 같구요.
flatmapLastest를 추가해서 여기서 작동시키는게 좋을 것 같습니다.
Comment on lines
+29
to
+37
| self.getUserProfileVisibility() | ||
| .map { $0.isProfilePublic } | ||
| .subscribe(with: self, onNext: { owner, isPublic in | ||
| owner.initStatus = isPublic | ||
| }) | ||
| .disposed(by: disposeBag) | ||
|
|
||
| self.isPublic = self.initStatus | ||
| print(self.isPublic, "💖") |
There was a problem hiding this comment.
W1
초기값을 init에서 굳이 실행할 필요가 없는 것 같아요 ! transform 메서드가 VC의 viewDidLoad에서 작동하니, transform 메서드 내에 넣어주면 더 일괄적이고 좋을 것 같습니다.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
⭐️Issue
close #233
🌟Motivation
비공개 여부를 수정하였습니다~!
🌟Simulation
🌟To Reviewer
get 은 되는데 patch 가 안됨
이유를 모르게쑴
우선 UI 까지만 머지할꼐여
++ 브런치 잘못파서 옮겨놓느라,,, 커밋이 뭉텅입니다 ,,,