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

[Fix] 모임일정 변경사항 대응 및 미비사항 수정 #211

Merged
merged 25 commits into from
Nov 13, 2024

Conversation

Seokki-Kwon
Copy link
Contributor

Related Issue

Description

모든기능을 확실히 잡아보려 했지만 모임일정에 생각보다 많은 서브피쳐들이 존재해서 기획재정비 기간에 꾸준히 리팩토링과 에러수정을 병행할 계획입니다 😭

  • 지도검색, 친구초대 화면을 연결하고 코디네이터를 재구성 했습니다.

    • 각화면을 연동하고 해당 서브피쳐들을 코디네이터에 위치시켜서 상태를 공유하도록 구현했습니다.
    • 기존의 단일 코디네이터에서 모임일정의 별도의 ChildCoordinator를 구성하도록 변경했습니다.
    • 액션체이닝을 통해서 다른 계층의 리듀서로 액션을 전달하는 방식으로, 직관적이고 확장 가능한 흐름을 구성했습니다.
  • TabView 컴포넌트를 추가했습니다.

  • 초대알림, 친구캘린더 부분은 따로 작업 예정입니다.

Screenshot (Optional)

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-11-06.at.15.28.41.mp4
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-11-06.at.15.29.06.mp4

Requirements for Review

작업을 진행하면서 부족함을 많이 느꼈습니다 작게보면 TCA와 같은 서드파티 라이브러리에 대한 숙련도는 물론, 앱 전반에서 공유되는 기능들의 모듈화 문제까지 개선이 필요함을 느꼈습니다.
기획재정비 기간이 생겼으니 해당 사항들을 보완하고 다음 작업을 진행해도 좋을 것 같습니다.

- AppCoordinatorView에 ErrorHandleAlert 제거(임시)
@Seokki-Kwon Seokki-Kwon added 🔧Fix 버그 수정 이슈 🔄Refactor 코드 리팩토링 이슈 labels Nov 6, 2024
@Seokki-Kwon Seokki-Kwon self-assigned this Nov 6, 2024
Copy link
Contributor

@iHyunWoo iHyunWoo left a comment

Choose a reason for hiding this comment

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

너무 양이 많아 전체를 보진 못하겠네요😅
TCA Coordinator를 사용한게 라우팅하는 부분을 분리해서 깔끔한 구조를 만드려한건데,
더 복잡해지는 느낌도 있네요 ㅎㅎ

@@ -30,16 +32,30 @@ public struct MoimSchedule: Decodable, Hashable {
self.kakaoLocationId = kakaoLocationId
self.participants = participants
}

public init() {
Copy link
Contributor

Choose a reason for hiding this comment

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

생성자를 중복 구현하기보단, 위의 init에 default 값을 넣어주는 것도 좋을 것 같네요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

엇 그러네요 ㅎㅎ
수정해서 반영하도록 하겠습니다!!

@Seokki-Kwon
Copy link
Contributor Author

너무 양이 많아 전체를 보진 못하겠네요😅 TCA Coordinator를 사용한게 라우팅하는 부분을 분리해서 깔끔한 구조를 만드려한건데, 더 복잡해지는 느낌도 있네요 ㅎㅎ

네..ㅎㅎ 더 깔끔하게 사용해보려해도 숙련도 문제도 있는것 같습니다
그래도 저 로직들이 각뷰에 존재하는 것보다는 나름 장점이 있는 것 같다고 느껴집니다.

Copy link
Contributor

@FpRaArNkK FpRaArNkK left a comment

Choose a reason for hiding this comment

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

으하 양 장난없네요 너무 고생하셨습니다!!
제 카카오 맵뷰를 컴바인과 함께 이쁘게 바꿔주셔서 감사합니더..

Comment on lines +29 to +36
// .namoAlertView(
// isPresented: Binding(get: { store.showAlert }, set: { store.send(.changeShowAlert(show: $0)) }),
// title: store.alertTitle,
// content: store.alertContent,
// confirmAction: {
// store.send(.doAlertConfirmAction)
// }
// )
Copy link
Contributor

Choose a reason for hiding this comment

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

해당 AlertView 경우는 문제가 아직 해결되지 않은 상태일까요??

Comment on lines +22 to +37
MoimCoordinator (루트 코디네이터)
├─ MainTab (메인 탭 화면)
│ └─ MoimList (모임 목록)
├─ MoimEditCoordinator (모임 수정 코디네이터)
│ └─ Sheet Presentation
│ ├─ Create Mode (새 모임 생성)
│ └─ Edit Mode (기존 모임 수정)
└─ Notification (알림 화면)
└─ Push Presentation

Flow:
1. 메인탭에서 모임 생성/수정 -> MoimEditCoordinator (Sheet)
2. 메인탭에서 알림 버튼 -> Notification (Push)
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

해당 구조 명시해두신거 너무 좋은 것 같아요..! bbb

Comment on lines +114 to +122
case .router(.routeAction(_, action: .kakaoMap(.backButtonTapped))):
if case var .createMoim(editStore) = state.routes[0].screen {
editStore.moimSchedule.locationName = state.placeSearchStore.locationName
editStore.moimSchedule.latitude = state.placeSearchStore.y
editStore.moimSchedule.longitude = state.placeSearchStore.x
editStore.moimSchedule.kakaoLocationId = state.placeSearchStore.id
state.routes = [.root(.createMoim(editStore), embedInNavigationView: true)]
}
return .none
Copy link
Contributor

Choose a reason for hiding this comment

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

와 이거 깔끔하고 좋네요..!
나중에 placeSearchStore -> editStore.moimSchedule의 mapper를 하나 만들어둬도 좋을 것 같아요!

Comment on lines +22 to +25
return .run { [state] send in
let response = try await friendUseCase.getFriends(page: 1, searchTerm: state.searchText)
await send(.searchResponse(response))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

혹시 해당 부분에서 state를 캡처하는 이유가 있을까요??

Comment on lines +72 to +76
deinit {
controller?.pauseEngine()
controller?.resetEngine()
}

Copy link
Contributor

Choose a reason for hiding this comment

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

와 마무리 좋네요

@FpRaArNkK FpRaArNkK merged commit 5426e76 into develop Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔧Fix 버그 수정 이슈 🔄Refactor 코드 리팩토링 이슈
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants