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

[#41] BusStopView 그리기 #48

Merged
merged 9 commits into from
Nov 5, 2024
Merged

Conversation

Minkyeong-Choi
Copy link
Collaborator

@Minkyeong-Choi Minkyeong-Choi commented Nov 1, 2024

📝 작업 내용

이번 PR에서 작업한 내용을 간략히 설명해주세요

  • 하이파이와 동일한 이름으로 가고자 뷰의 이름을 MapView -> BusStopView로 변경하였습니다.
  • BusStopView를 하이파이에 맞게 그립니다.
  • 이번 정류장 뷰는 ThisStopView, 목적지 정류장 뷰는 EndStopView로 파일을 새로 만들었습니다.
  • 사용자의 현재 위치를 나타내는 파란색 마커를 커스텀하여 하이파이에 있는 이미지로 변경하였습니다.

스크린샷 (선택)

💬 리뷰 요구사항(선택)

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

  • 이미지, 폰트 에셋이 머지되면 이후 추가할 예정입니다.
  • ThisStopView의 배경을 현재는 ultraThinMaterial 설정으로 처리해놓았는데, 이후 blur를 적절하게 입힐 수 있는 방법을 찾으면 바꿀 예정입니다. (새로운 이슈로 작업할 예정)
  • BusStopView로 진행하는 것이 좋은지, 아니면 기존대로 MapView로 진행하는게 좋은지 궁금합니다! 어떤 기준에 맞추어야할지 잘 모르겠어요.

@Minkyeong-Choi Minkyeong-Choi self-assigned this Nov 1, 2024
@Minkyeong-Choi Minkyeong-Choi linked an issue Nov 1, 2024 that may be closed by this pull request
5 tasks
Comment on lines 12 to 13
var xCoordinate: Double
var yCoordinate: Double
Copy link
Collaborator

Choose a reason for hiding this comment

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

저희 지난번에 정했던 것처럼 latitude, longitude로 수정 부탁드립니다 ~

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

수정 완료했습니다~!

Comment on lines +17 to +18
@ObservedObject var locationManager: LocationManager
@ObservedObject var busStopSearchViewModel: BusStopSearchViewModel
Copy link
Collaborator

Choose a reason for hiding this comment

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

현재 저희 view 계층이, search -> map으로 이루어져 있습니다.
그리고 두 화면에서 모두 딱 이 두 개의 모델을 필요로 해요. (LocationManager, BusStopSearchViewModel)

그래서 최상위 view인 searchView에 @EnvironmentObject로 선언해두고 하위 view에 주입해서 사용하려고 해요~

이 부분은 현재 PR까지 반영되고, view 연결하는 과정에서 제가 수정하겠습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵!

struct BusStopView: View {
@ObservedObject var locationManager: LocationManager
@ObservedObject var busStopSearchViewModel: BusStopSearchViewModel
@State private var items: [Coordinate] = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

items보다는 어떤 데이터를 담고 있는지 명확하게 나타내는 변수명이면 좋을 것 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

수정완료하였습니다!

Comment on lines +33 to +34
.padding(.trailing, 15.48)
.padding(.top, 23.91)
Copy link
Collaborator

Choose a reason for hiding this comment

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

혹시 Figma에서도 padding 값이 소수점으로 나와 있나요 🤔??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

네 맞습니당

@dbqls200
Copy link
Collaborator

dbqls200 commented Nov 3, 2024

BusStopView로 진행하는 것이 좋은지, 아니면 기존대로 MapView로 진행하는게 좋은지 궁금합니다! 어떤 기준에 맞추어야할지 잘 모르겠어요.

hmm .. 해당 뷰가 나타내는 걸 생각했을 때는 BusStopViewMapView 모두 조금 아쉬운 느낌이 드는 것 같네요 ..
이름에서 어떤 화면인지 잘 드러나지 않는달까 ..

그치만 현재 하이파이에서 해당 뷰를 BusStopView으로 표현하고 있다면 통일해봅시다 !
디자인팀과 소통할 때 더 수월할 것 같아요 😎

Copy link
Collaborator

@dbqls200 dbqls200 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다 ~

@Minkyeong-Choi Minkyeong-Choi merged commit b11cc5f into dev Nov 5, 2024
@Minkyeong-Choi Minkyeong-Choi deleted the fix/41-editting-map-view branch November 6, 2024 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MapView 하이파이에 맞게 수정하기
2 participants