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

[feature][#19] 주종 선택 UI #40

Merged
merged 11 commits into from
Sep 3, 2024
Merged

Conversation

dayo2n
Copy link
Collaborator

@dayo2n dayo2n commented Aug 21, 2024

Descriptions

  • 주종 선택 UI를 개발합니다

리뷰가 필요한 부분

  • 많을듯 ㅎㅎ! 😭
  • 뷰 구조를 제대로 만든건지 모르겠어요: 쓸데없이 stack view에 담는다던지... 반대로 스택에 담을걸 다 따로 선언했다던지 ?
  • DrinkSelectionViewModel.DrinkType이랑 DrinkType 열거형 통일 못했음!!! 오빠가 MiniCard에서 쓰고 있던데 인자로 받기만 하고 어디 쓰지는 않아서 요 부분 필요없어지면 그냥 DrinkType을 지우면 될 것 같은데 나중에 필요한 부분이면 코멘트줘 취소 ... 뷰모델에 종속되지 않고 DrinkType 꺼내서 써야겠당

Simulator Screen Recording - iPhone 15 Pro - 2024-08-27 at 16 41 24

@dayo2n dayo2n linked an issue Aug 21, 2024 that may be closed by this pull request
1 task
@dayo2n dayo2n marked this pull request as draft August 21, 2024 14:42
@enebin
Copy link
Member

enebin commented Aug 23, 2024

리뷰를 해도 되는 것인가요?!

@enebin
Copy link
Member

enebin commented Aug 25, 2024

문 열어~ 문 열어~ 문 열어~ 문 열어~ 문 열어~ 문 열어~ 문 열어~ 문 열어~

@dayo2n dayo2n marked this pull request as ready for review August 27, 2024 01:18
@dayo2n
Copy link
Collaborator Author

dayo2n commented Aug 27, 2024

ㅋㅋㅋㅋㅋㅋㅋ쏘리 ㅎ 아마 마지막 커밋으로 마무리했던거같은디 오후에 잠시 살펴보겠음 !!!

@dayo2n dayo2n requested a review from enebin August 27, 2024 06:56
@dayo2n dayo2n self-assigned this Aug 27, 2024
@dayo2n dayo2n added the ✨ feature New feature or UI label Aug 27, 2024
@dayo2n
Copy link
Collaborator Author

dayo2n commented Aug 27, 2024

@enebin 작은 이슈가 있어 수정했습니다요 !!!! 매운 리뷰 부탁해 ....

Copy link
Member

@enebin enebin left a comment

Choose a reason for hiding this comment

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

너무 수고 많았습니다!!!!!

코멘트가 어쩌다보니 많아졌는데 크리티컬한 문제는 업슴.
대부분 개인 의견 위주라 보시고 필요한 것만 반영해주심 될 것 같읍니다.

고생 많았어!@!@#!@!@!

view.addSubview(typeTitleImageView)
typeTitleImageView.snp.makeConstraints { make in
make.centerX.equalToSuperview()
make.top.equalTo(view.safeAreaLayoutGuide)
Copy link
Member

Choose a reason for hiding this comment

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

얘가 child 뷰 컨트롤러라 top constraint을 결정할 권한은 없는 것 같긴 하지만.. (ex. 부모 뷰컨에서 top을 화면의 센터로 잡으면..?)

Suggested change
make.top.equalTo(view.safeAreaLayoutGuide)
make.top.equalToSuperView()
// 부모 뷰컨에서
view.addSubview(pageViewController.view)
pageViewController.view.snp.makeConstraints { make in
    make.top.left.trailing.equalTo(view.safeAreaLayoutGuide)
    make.bottom.equalToSuperview()
  }

사실 원칙대로면 이런식으로 부모 뷰컨에서 safeAreaLayoutGuide를 잡아주는게 맞긴 합니당.
근데 딱히 재사용이 되는 뷰는 아니라 그냥 놔둬도 될듯?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

흐흐아암 safearea로 안잡아주면 넘어가버리는데 ... 이렇게 되는게 이상한거야 ??

스크린샷 2024-09-03 오후 8 10 29

@dayo2n dayo2n merged commit 20c5087 into develop Sep 3, 2024
@dayo2n dayo2n deleted the feature/19-choose-type-UI branch September 3, 2024 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feature New feature or UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

기록 - 주종선택 UI
2 participants