Skip to content

Conversation

@anjiniii
Copy link
Contributor

@anjiniii anjiniii commented Apr 4, 2025

About this PR

🔖 Related Issue


📚 Contents

아래 체크된 기능을 구현했습니다.
다음 내용이 많아질 것 같아서, 작은 내용 먼저 PR 리뷰를 부탁합니다!
내용이 많지 않습니다~

  • 컴피존 설정 화면 초기 화면 구현
  • 컴피존 관련 정보 팝업
  • 위치 권한이 없을 때, 권한 요청 팝업
  • 위치 권한 있을 때
    • 컴피존 설정 + 텍스트필드 활성화
    • 컴피존 삭제 (컴피존 안/밖)

📸 Screenshot

image image

Comment on lines +10 to +28
extension View {
@ViewBuilder
func comfieZoneInfoPopupView(
showPopup: Bool,
onDismiss: @escaping () -> Void
) -> some View {
ZStack {
self

if showPopup {
Color.popupBackdrop.ignoresSafeArea()
.onTapGesture { onDismiss() }

ComfieZoneInfoPopupView(dismissPopup: onDismiss)
.padding(.horizontal, 50)
}
}
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

View 내부의 코드가 복잡해지는 것을 막고자, 컴피존 정보 팝업을 modifier로 구성했습니다.

@anjiniii anjiniii added Feature 🍄 리뷰해주세요 ☃️ 구현 완료! 리뷰 부탁! and removed 작업중이에요 🏃‍♀️‍➡️ 작업중! 리뷰 아직! labels Apr 9, 2025
@anjiniii anjiniii changed the title ✨ Feat: 컴피존 설정 화면 구현 (오직 화면뿐) ✨ Feat: 컴피존 설정 초기 화면 구현 (오직 화면뿐) Apr 9, 2025
@anjiniii anjiniii requested review from Guryss and zaehorang April 9, 2025 11:43
"color-space" : "extended-srgb",
"components" : {
"alpha" : "0.660",
"alpha" : "0.400",
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
Contributor

Choose a reason for hiding this comment

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

음? 디자인시스템엔 66이라고 되어있는데 중간에 바뀌었나보네용 ㅠ

Copy link
Contributor

@Guryss Guryss left a comment

Choose a reason for hiding this comment

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

수고하셨습니닷! 확인해야할 지점들이 몇가지 있는 것 같습니당!! 화이팅그팅그

"color-space" : "extended-srgb",
"components" : {
"alpha" : "0.660",
"alpha" : "0.400",
Copy link
Contributor

Choose a reason for hiding this comment

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

음? 디자인시스템엔 66이라고 되어있는데 중간에 바뀌었나보네용 ㅠ

)
}
.background(Color.cfWhite)
.cfNavigationBar("현재위치", trailingButtons: [infoButton])
Copy link
Contributor

Choose a reason for hiding this comment

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

지금 실기기로 돌려보니 navigationBar 영역이 자리를 잡고 있는데 확인 필요할 것 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

나이스캐치!!!!
실기기에서 실행했고, 문제 해결했습니다! 네비게이션바 상위에 View를 만들고 있어서 생기던 문제였습니다 ㅎ하하

.disabled(true)

// 컴피존 안내 문구 - 안, 밖, 없음
Text("컴피존이 없어요!")
Copy link
Contributor

Choose a reason for hiding this comment

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

String 추출 필요하네요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아직 구현하지 않은 View 입니다! 추후에 다른 View로 추출해서 활용할 것 같아요!
참고할 수 있도록 언급했어야 하는데, 죄송함니당ㅠ


// 컴피존 설정 시트
VStack(alignment: .leading, spacing: 16) {
Text("컴피존")
Copy link
Contributor

Choose a reason for hiding this comment

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

여기도! String 추출

Copy link
Contributor Author

Choose a reason for hiding this comment

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

여기도 아직 구현하지 않은 View 입니다! 추후에 다른 View로 추출해서 활용할 것 같아요!

)
}
.background(Color.cfWhite)
.cfNavigationBar("현재위치", trailingButtons: [infoButton])
Copy link
Contributor

Choose a reason for hiding this comment

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

여기도! String 추출

Copy link
Contributor Author

Choose a reason for hiding this comment

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

여기는 추출했습니다!

Comment on lines +20 to +21
Color.popupBackdrop.ignoresSafeArea()
.onTapGesture { onDismiss() }
Copy link
Contributor

Choose a reason for hiding this comment

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

백그라운드 터치했을 때에도 dismiss 되야하는지 함 물어봐야할 것 같은데용?
아니면 논의가 된 부분인지 궁금합니닷! 피그마 기능 명세에는 없어서요 !

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아하 팝업이라 너무 당연하게 dismiss 된다고 생각했네요! 물어보겠습니다~

Copy link
Contributor Author

@anjiniii anjiniii Apr 9, 2025

Choose a reason for hiding this comment

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

X버튼 또는 검은 배경 탭할 때 팝업이 사라지게 해주세용!!

라고 답변 받았습니다~ 여기 코드는 그대로 둘게요!

Copy link
Contributor

@zaehorang zaehorang left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~
구리스가 언급해주신 navigationBar 문제만 해결하면 될 거 같아요❗️

ZStack(alignment: .topTrailing) {
VStack(spacing: 20) {
// 타이틀 - 컴피존이란?
Text(strings.title)
Copy link
Contributor

Choose a reason for hiding this comment

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

localized 처리를 해줘야 하는 거 같은데 맞나요 ?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오 아뇨! Text는 LocalizedStringResource를 그대로 받아서 처리할 수 있기 때문데 localized 처리를 하지 않아도 괜찮습니다!

참고로 WWDC 영상에서 "Localization을 위해 LocalizedStringResource를 사용하도록 권장되고, 이 타입은 리터럴뿐만아니라 comment, table, key와 다른 default value를 지정할 수 있다." 는 언급이 있습니다!
https://developer.apple.com/videos/play/wwdc2023/10155/?time=1368

Copy link
Contributor

Choose a reason for hiding this comment

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

image

오..! 저는 이 코드가 꼭 필요한 로직이라고 생각했는데,
말씀하신 걸 보니 이 계산 프로퍼티는 String 타입만 필요한 경우에 한정해서 사용하기 위한 거였군요!
(제가 이해한 게 맞을까요? 😅)

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
Contributor

Choose a reason for hiding this comment

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

오호.. 그러면 저희가 추가하는 커스텀 뷰에도 String이 아닌 LocalizedStringResource로 선언해서 사용하는게 더 깔끔하겠네유 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오 그렇네요! 그렇게 생각을 못해봤는데, 커스텀 뷰에도 그렇게 활용하면 좋을 것 같아요!


struct ComfieZoneSettingView: View {
@State var intent: ComfieZoneSettingStore
private var state: ComfieZoneSettingStore.State { intent.state }
Copy link
Contributor

Choose a reason for hiding this comment

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

미리 선언해두고 쓰는거 좋아 보입니다 👍

Comment on lines -12 to 23
let router: Router
// TODO: MemoRepository를 어디서 주입할 지 고민하기

// MARK: - Repository
let memoRepository: MemoRepositoryProtocol = MemoRepository()

// MARK: - Intent
private func makeOnboardingIntent() -> OnboardingStore { OnboardingStore(router: router) }
private func makeMemoIntent() -> MemoStore {
MemoStore(router: router, memoRepository: memoRepository)
MemoStore(
router: router,
memoRepository: memoRepository
)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

수정 감사합니당 🐯

Copy link
Contributor

@zaehorang zaehorang left a comment

Choose a reason for hiding this comment

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

확인했습니다 👨🏻‍⚖️

@Guryss Guryss self-requested a review April 13, 2025 06:55
Copy link
Contributor

@Guryss Guryss left a comment

Choose a reason for hiding this comment

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

수고하셨습니당 !!

@anjiniii anjiniii merged commit d015191 into develop Apr 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants