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

1.0.0에서 발견한 버그 수정 #114

Merged
merged 6 commits into from
Sep 8, 2024
Merged

Conversation

ShapeKim98
Copy link
Contributor

@ShapeKim98 ShapeKim98 commented Sep 7, 2024

#️⃣연관된 이슈

#111

📝작업 내용

이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)

  • 컨텐츠 상세에서, 즐겨찾기와 컨텐츠 조회 액션이 발생했을 시, 부모 뷰 에게 전달하는 시점을 변경하였습니다.
  • 컨텐츠 공유로 공유 시트를 띄웠을 시에 공유시트의 닫기 버튼을 눌렀을 시에 닫히지 않는 문제를 수정하였습니다.
  • 구글 로그인 회원 탈퇴 시 우리 서버로 탈퇴 요청이 실행되지 않는 문제를 수정하였습니다.
  • 회원가입 완료 시 뒤로가기 버튼을 없애기 위해 LoginRootFeatureLoginFeature <-> SignUpFeatureifCaseLet 처리로 바꿨습니다.

스크린샷 (선택)

Simulator Screen Recording - iPhone 15 Pro - 2024-09-07 at 19 03 53

Simulator Screen Recording - iPhone 15 Pro - 2024-09-07 at 19 04 48

Simulator Screen Recording - iPhone 15 Pro - 2024-09-07 at 19 07 20

💬리뷰 요구사항(선택)

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

  • 앱 멈춤 현상을 해결하기 위해 컨텐츠 상세에서 부모 뷰에 액션 전달 시점을 바꿔보았지만, ifLet오류만 사라졌을 뿐 증상은 동일하였습니다. (제 생각에는 TCA의 고질적 문제가 아닐까...)
  • 어쩌다 보니 구글로그인 회원탈퇴가 안되는 것을 발견하여 해결하였습니다. (안고치기엔 너무 치명적인거 같았어요)
  • 회원 탈퇴 고치면서 회원가입 완료 화면에 뒤로가기를 비활성화 하였습니다.
  • 굳이 굳이 힘들게 ifCaseLet로 바꾼 이유는 저희가 뒤로가기 버튼을 커스텀 하면서, 뒤로가기 제스처를 강제로 활성화 시켰기 때문입니다. 이로 인해 버튼이 안보여도 뒤로가기가 가능해지는 문제를 보였고, 아예 화면 플로우를 자체를 분리 시켰습니다.
  • ifCaseLet 처리를 하다가 문득 든 생각은.. Pokit <-> Remind 화면간 전환에 TabView를 쓸 필요가 있나? 싶었습니다. 어떻게 생각하시는지 궁금해요.

close 이슈번호

@ShapeKim98 ShapeKim98 added Fix 기능 수정 Bug 🔫 현재 발견된 버그를 수정하기 위함 labels Sep 7, 2024
@ShapeKim98 ShapeKim98 self-assigned this Sep 7, 2024
@ShapeKim98 ShapeKim98 changed the title Fix/#111 sheet bug 1.0.0에서 발견한 버그 수정 Sep 7, 2024
@stealmh
Copy link
Member

stealmh commented Sep 8, 2024

고생많으셨습니다! 질문주신것만 확인하고 천천히 살펴볼게요.

  • 화면 전환 간 TabView를 사용해야 할까?
    메인탭피쳐가 두개의 자식들을 가지고 있는데 (포킷, 리마인드) 이들간의 전환이 필요할 때
    TabView를 사용하라고 공식문서에 나와있는데, 저도 크게 고민해본 적은 지금까진 없었던 것 같아요 어떤 이유 때문인지 듣고싶습니다-!

Comment on lines 222 to 224
case .링크_공유_완료(completed: let completed):
state.shareSheetItem = nil
return .none
Copy link
Member

Choose a reason for hiding this comment

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

연관값이 사용이 안되는데 어떤 경우에 쓰시려고 다신 건지 궁금합니다

Copy link
Contributor Author

Choose a reason for hiding this comment

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

연관값이 사용이 안되는데 어떤 경우에 쓰시려고 다신 건지 궁금합니다

아 이거 전에 공유가 됐는지 안됐는지에 따라 시트를 닫을려고 썼던 연관값인데, 생각해보니 이제 필요 없어졌네요... 지우겠습니다!

Copy link
Member

Choose a reason for hiding this comment

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

저였으면 회원가입 완료를 @Presents로 바꾸는 작업만 따로 진행할 것 같아요.
fullScreenCover로 띄워버리는 쪽이 수정이 덜 할 것 같아서요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

저였으면 회원가입 완료를 @Presents로 바꾸는 작업만 따로 진행할 것 같아요. fullScreenCover로 띄워버리는 쪽이 수정이 덜 할 것 같아서요

사실 그걸 생각 안해본건 아닌데.. 그냥 사용자 입장에서 화면 전환이 더 깔끔하다고 느껴서 이렇게 작업했습니다..!! 사실 TCA의 delegate과 캡슐화 덕분인지 작업하는데 큰 무리는 없었어여

Copy link
Member

Choose a reason for hiding this comment

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

ㅇㅈ 사실취향차이

@@ -332,7 +332,6 @@ private extension PokitSearchFeature {
state.domain.condition.isRead = false
return .send(.inner(.페이징_초기화))
case .링크_공유_완료(completed: let completed):
guard completed else { return .none }
Copy link
Member

Choose a reason for hiding this comment

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

리뷰하다가 봤는데 수정되면서 completed가 미사용되는거군여! 이런경우도 수정하면서 같이 지워주세요!

Copy link
Member

Choose a reason for hiding this comment

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

👍👍👍👍👍👍

@stealmh
Copy link
Member

stealmh commented Sep 8, 2024

일단 apporve해둘테니 수정하고 합쳐주시면 될 것 같아요 굿짭!

@ShapeKim98
Copy link
Contributor Author

일단 apporve해둘테니 수정하고 합쳐주시면 될 것 같아요 굿짭!

감사합니다!

@ShapeKim98
Copy link
Contributor Author

고생많으셨습니다! 질문주신것만 확인하고 천천히 살펴볼게요.

  • 화면 전환 간 TabView를 사용해야 할까?
    메인탭피쳐가 두개의 자식들을 가지고 있는데 (포킷, 리마인드) 이들간의 전환이 필요할 때
    TabView를 사용하라고 공식문서에 나와있는데, 저도 크게 고민해본 적은 지금까진 없었던 것 같아요 어떤 이유 때문인지 듣고싶습니다-!

저희가 커스텀 탭바를 쓰면서 기본 탭바에 커스텀 탭바를 덮어 씌우는 형식으로 구현을 하다 보니 이런 생각이 들었습니다..!!

@ShapeKim98 ShapeKim98 merged commit b72df49 into develop Sep 8, 2024
1 check passed
@stealmh stealmh deleted the fix/#111-sheet-bug branch September 9, 2024 01:06
@stealmh stealmh linked an issue Oct 29, 2024 that may be closed by this pull request
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🔫 현재 발견된 버그를 수정하기 위함 Fix 기능 수정
Projects
None yet
Development

Successfully merging this pull request may close these issues.

정식 출시 전 발견한 버그
2 participants