목표 삭제 기능 구현 #111
목표 삭제 기능 구현 #111
Conversation
다시보니 굳이 viewWillAppear(_:)에 있을 필요가 없음
- swipe의 경우 테이블 뷰에 비해 컬렉션 뷰는 iOS14 이후로 Compositional Layout 내부에 적용할 수 있기 때문에 우선은 메뉴버튼으로 대체
…수 추가해서 좀 더 의미 분명하게 수정
| override func viewWillAppear(_ animated: Bool) { | ||
| super.viewWillAppear(animated) | ||
|
|
||
| viewModel?.requestGoalList.accept(()) |
There was a problem hiding this comment.
제드 이거 GoalDetailVC에서 사진을 찍고 GoalListVC로 돌아왔을 때 Goal.progress가 화면에 반영이 안돼서 viewWillAppear에 위치시켰었는데, 혹시 이렇게 제거하면 해당 문제없었나요??
There was a problem hiding this comment.
다시 확인해볼께요 ㅋㅋ 우선은 뷰모델 관련 로직 한군데로 몰아넣어보려고 이동했던거였어요!
|
|
||
| viewModel.requestGoalList.accept(()) | ||
|
|
||
| viewModel.showActionSheetMenu |
There was a problem hiding this comment.
저 저번에 "공유기능" 만들 때 이런 부분에 대해서 고민해본적이 있었는데, UIViewController.present(_:_:) 메소드(화면이동) 사용이 필요한 부분이라 VC에서 할지, Coordinator에서 할지 결정하기 쉽지 않더라구요ㅋㅋ 제드는 혹시 어떻게 생각하셨나요?
There was a problem hiding this comment.
음.. 독립적인 화면으로 관리할꺼면 코디네이터로 가는게 맞고,
모든 상황에서 컬렉션 뷰 셀같이 하위 화면으로 종속되는거면 뷰컨트롤러에 있어도 괜찮겠다고 생각했어요.
액션 시트같은 경우에는 별도의 뷰컨트롤러는 아니어서 그냥 저렇게 안에 넣어놨습니다 ㅋㅋ
There was a problem hiding this comment.
@Hansolkkim
이거 주말동안 다시 한 번 생각해봤는데,
모든 Alert 포함 화면에 present하는 책임을 Coordinator에 주고 싶으면
- Alert 보여주는 디폴트 메소드를 가진 프로토콜 하나 선언
- NavigationController을 가진 Coordinator이 해당 프로토콜 채택 후, 필요할 때마다 NavigationController 위에서 Alert 보여주는 디폴트 메소드 호출
이런식으로도 개선 가능하겠네요..!
(테스트해보고 커밋 올려볼께요!)
| make.top.equalToSuperview() | ||
| make.bottom.equalTo(titleStackView.snp.top) | ||
| make.trailing.equalToSuperview().inset(20/376 * UIScreen.main.bounds.width) |
There was a problem hiding this comment.
레이아웃 관련 제안이라 조금 애매하지만, menuButton의 위치가 뭔가 위에만 붙어있는 느낌이라, Top, Trailing Margin이 일치하면 더 보기 좋을 것 같다는 생각이 들었어요!
There was a problem hiding this comment.
듣고보니 위에도 간격주는게 더 보기 좋겠네요 ㅎㅎ 이것도 수정할께요
There was a problem hiding this comment.
top margin 이 없을 때
top margin 이 trailing margin과 동일할 때
이런식으로 버튼의 레이아웃이 다르게 나타납니다. 처음에는 제안해주신대로 top margin에 값을 부여하면 기존과 높이는 유지된채로 셀의 top에서부터 약간 떨어진 정도가 되겠구나 생각했는데, 해놓고보니 위처럼 나타나더라구요. bottom anchor은 유지한 상태에서, top margin값만 부여하다보니 자동으로 높이가 조정되서 저렇게 보인 것 같습니다.
SF Symbol에서 제공하는 기본 아이콘을 우선 가져온 상황에서, 이미지를 감싸는 배경이 되는 버튼의 높이를 넉넉하게 잡으려고 셀의 top과 셀 내부의 titleStackView의 top의 사이 간격 전체를 차지하도록 저렇게 높이를 잡았었는데(이미지의 크기는 버튼 높이와 무관하게 동일합니다.), 우선은 이 부분은 기존대로 두는게 괜찮을 것 같습니다! top margin이 들어가면 버튼의 높이가 줄어들어서 아무래도 사용자가 메뉴를 보기 위해 터치해야 할 영역이 너무 작아지는 것 같아서요
조만간 이 부분도 디자인 바뀌거나 다른 아이콘을 넣기로 합의하게 되면 그 때 디자인 맞춰서 레이아웃 다시 조정하면 될 것 같습니다 ㅎㅎ
| .flatMapLatest { viewModel, goal in | ||
| viewModel.repository.deleteGoal(goal: goal) | ||
| } |
There was a problem hiding this comment.
deleteGoal에 대한 바인딩 2번을 한 번에 하기 보다는, result에 대한 바인딛 1번, 그리고 result의 결과에 대한 바인딩 1번, 이렇게 2번으로 나누어서 바인딩 로직을 짜는건 어떨까요?
let deleteResult = deleteGoal
.withUnretained(self)
.flatMapLatest { viewModel, goal in
viewModel.repository.deleteGoal(goal: goal)
}
.disposed(by: disposeBag)
deleteResult
.withUnretained(self)
.bind {
...
}
.disposed(by: disposeBag)There was a problem hiding this comment.
제안해주신대로 수정했습니다 ㅎㅎ 훨씬 직관적이네요
close #108
(좀 보기 이상한 UI다 싶으면 같이 얘기해보고 기능 유지한채로 디자인만 다시 수정하면 될듯합니다!)