Skip to content

Conversation

@snughnu
Copy link
Contributor

@snughnu snughnu commented May 2, 2025

배경

스크린샷 2025-05-02 16 34 57

작업 내용

UI.mov

지도

  • 지도는 MKMapView를 올려놓기만 한 상태입니다.
  • 지도를 설정하는 부분은 따로 PR올리겠습니다.

바텀시트

  • grabber, 내 위치 버튼, 테이블뷰로 구성되어있습니다.

필터 버튼

  • 스크롤뷰 위에 버튼들을 배치했습니다.

테스트 방법

  • rootView를 변경하여 테스트할 수 있습니다.
  • 지도의 확대/축소는 더블클릭/더블클릭의 두번째는 누른 상태로 위아래로 커서 이동으로 할 수 있습니다.

리뷰 노트

  • 단순 UI만 구현하였으며, 네트워킹 및 추가 설정 로직은 추후에 있을 예정입니다.

@snughnu snughnu added the 🎨 Feature 기능 구현 label May 2, 2025
@snughnu snughnu self-assigned this May 2, 2025
Copy link
Member

@MinwooJe MinwooJe left a comment

Choose a reason for hiding this comment

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

테이블 뷰 어려우셨을텐데 고생많으셨습니다!

그런데 searchBottomView와 뷰 컨트롤러를 분리한 이유가 있을까요?
개인적으로는 둘을 통합하는게 더 나을 것 같습니다.
재사용이 되는 컴포넌트가 아니라서 뷰룰 굳이 뺄 필요도 없고, 지금처럼 분리한다면 접근 제어자(private) 때문에 코드가 조금 더 복잡해지기도 하구요!

혹시 다른 생각 있으시다면 말씀해주세요!!

config.baseForegroundColor = UIColor(.black)
config.imagePadding = 4
config.contentInsets = NSDirectionalEdgeInsets(top: 7, leading: 14, bottom: 7, trailing: 14)
config.cornerStyle = .capsule
Copy link
Member

Choose a reason for hiding this comment

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

p1)
cornerStyle을 capsule로 지정했는데, cornerRadius까지 설정하니 UI 처리가 중복되는 것 같습니다!
둘 중 하나만 사용하는게 좋을 것 같애요!

참고로 Source/Util/Extensions/UIView+ 파일에 UIVIew의 익스텐션으로 cornerRadius() 메서드를 추가해뒀습니다!
masksToBounds까지 처리되니 저 메서드를 사용하는게 좋을 듯 합니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

extension 활용해서 수정했습니다~

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.

처음에는 캡슐은 버튼의 실제 높이가 확정된 후(오토레이아웃 이후)에만 설정할 수 있고
intrinsicContentSize.height를 사용하면 초기화 시점에서도 정확하게 cornerRadius 설정이 가능하기 때문에
레이아웃 타이밍에 영향을 받지 않도록 cornerRadius를 사용했습니다

이유를 생각하다 lazy var대신 let을 쓸 수 있어서 바꿨습니다!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

intrinsicContentSize를 사용해서 cornerRadius를 설정하더라도
오토레이아웃을 잡을 때 버튼의 높이가 바뀌어버리면 캡슐모양이 안나오는 문제가 있을 수 있다는 점을 해결하기 위해

layoutSubviews뷰의 생명주기 메서드를 사용해서 캡슐모양을 설정했습니다!

override func layoutSubviews() {
    super.layoutSubviews()
    myLocationButton.configuration?.cornerStyle = .capsule // 적용 안됩니다!!
}

여기서도 .capsule을 사용하지 않은 마찬가지 이유로
.capsule은 버튼의 크기(오토레이아웃)가 결정되기 전에 설정되므로, layoutSubviews에서 사용해도 모양이 반영되지 않기 때문입니다!

@snughnu
Copy link
Contributor Author

snughnu commented May 2, 2025

뷰와 뷰컨 분리의 질문에서 뷰와 뷰컨을 합쳤을 때 재사용성이 떨어지는 문제는 해결되지만,
뷰컨이 무거워지는 것, 코드의 구조가 일관되지 않는 것, 가독성이 떨어지는 것을 생각했을 때는 분리하는게 맞다고 생각됩니다.

@MinwooJe
Copy link
Member

MinwooJe commented May 2, 2025

일반적으로 뷰와 뷰컨을 합치면 재사용성이 향상되는 것이 아닌 재사용성이 떨어집니다!

뷰와 뷰컨 분리에 대해 더 이야기해보면 좋을 것 같애요!!
MVVM의 View는 보통 뷰 컨트롤러가 수행하기에 아키텍처 상의 문제는 없습니다.

뷰컨이 무거워지는 것은 동의하지만, 코드의 구조과 일관되지 않는다는 점은 어떤 부분에서 그런것일까요?
마찬가지로 뷰의 멤버에 private을 붙여 public interface를 만드는 것은 오히려 가독성을 떨어뜨릴 수도 있지 않을까요?ㅎㅎ

저는 일반적으로 재사용 컴포넌트가 아니라면 뷰를 굳이 분리하지 않는다라는 기준이 있습니다.
MVVM에서 뷰컨은 어차피 뷰의 역할을 하고, 하나의 클래스안에 모아두면 관리나 가독성 측면에서 장점이 있다고 생각하기 떄문에요.

뷰와 뷰컨을 분리할 조금 더 명확한 근거가 있으면 좋을 것 같애요!

혹시 직감적으로 뷰와 뷰컨을 분리하는게 맞다고 생각되시면 그대로 구조를 유지해보시고 장단점을 생각해보는 것도 좋은 경험일 것 같습니다!!

Comment on lines 40 to 42

let height = button.intrinsicContentSize.height
button.layer.cornerRadius = height / 2
button.isEnabled = false
button.cornerRadius(radius: height / 2)
Copy link
Member

Choose a reason for hiding this comment

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

수정된 코드에서 height를 intrinsicContentSize 기준으로 잡고 있는데, view의 frame이나 bound를 사용하지 않은 이유도 궁금합니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

초기화 시점에는 button.frame.height가 0이니깐 사용할 수 없고,
intrinsicContentSize.height는 버튼의 고유 사이즈니깐 cornerRadius 설정에 적합해서 사용했습니다~

@snughnu
Copy link
Contributor Author

snughnu commented May 2, 2025

“재사용성이 떨어지는 문제가 해결된다"고 잘못 말해버렸네요...ㅎㅎ
아키텍처 관점에서 View 계층이기 때문에 MVVM 구조상 문제가 없다는 점에는 동의합니다!

구조의 일관성 이야기는 다른 화면에서는 View를 따로 분리해뒀는데, 특정 화면에서만 ViewController 내에서 UI를 구성하게 되면 코드 스타일이 일관되지 않는다는 얘기였습니다. (컨벤션으로 정해두진 않았지만 제 개인적인 코드 규칙에서 그렇습니다.ㅎㅎ) 근데 앞으로는 뷰/뷰컨 분리 기준에 프로젝트의 코드 일관성은 안넣으려고 합니다!

가독성 관련해서는 public interface만으로도 가독성이 안좋은데, 앞으로 ViewModel과의 바인딩이나 로직이 추가될 경우 더 가독성이 안좋아질 수 있다는 점을 고려한 말이었습니다.

그리고 가장 중요할거 같은 이유로 기획-디자인이 완전히 정해지진 않은거 같지만, 바텀시트에서 초기화면, 검색결과 화면, 상세화면에서 View 재사용 가능성이 높아 보여사 뷰/뷰컨을 분리했습니다!!

@MinwooJe
Copy link
Member

MinwooJe commented May 2, 2025

넵 성훈님 의견에 납득됐습니다!! 감사합니다~~

@snughnu snughnu merged commit db726ae into GDSC-Popcorn:develop May 4, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🎨 Feature 기능 구현

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants