-
Notifications
You must be signed in to change notification settings - Fork 33
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
박스오피스II [STEP 2] BMO, Serena #108
base: ic_9_serena
Are you sure you want to change the base?
Conversation
setCalendarRange는 limitCalendarRange로 변경
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@serena0720 @bubblecocoa
이번에도 스텝 2 고생 많으셨습니다ㅎㅎ
저번부터 느끼지만 잘 구현해주셔서 크게 코멘트 드릴게 없네요 😁
이번에도 각 리스트, 아이콘 셀 들을 잘 잡아주시고 코드로 잘 구현해주셔서 코드적으로 수정될 부분은 보이지 않았습니다.
빠르게 다음 스텝을 위해 확인하는 정도만 거쳐도 좋을듯합니다!
칭찬 드리고 싶은 부분
- 세심한 사용성 디테일까지 잡아주신점 아주 좋아요 💯
- 각 셀을 잘 만들어 활용해주셨네요 💯
보완되면 좋을 부분
- 현재 리스트 -> 아이콘으로 뷰모드가 전환될때 아이콘 셀들의 최상단이 아닌 3순위 정도부터 걸쳐져서 보이고 있는것 같아요!
이 부분이 화면 전환 시 offset이 1,2 순위부터 최상단 보이도록 하면 어떨까요~?
정말 잘 구현해주셔서 어프로브 해두겠습니다!
필요하시면 바로 머지 요청해주세요 ㅎㅎ
|
||
import Foundation | ||
|
||
extension UserDefaults { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오... 디테일한 부분까지 신경쓰셨군요 👍
BoxOffice/Model/ViewMode.swift
Outdated
case list | ||
case icon | ||
|
||
var anotherOption: String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
anotherOption이란 네이밍이 조금 어색해보이는데 탄생하게된 배경이 궁금합니다! 🙋🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7023922
해당부분 저희도 어색해하던 부분이라 좀 더 고민 후 수정했습니다 😄
} | ||
|
||
private func configureUI() { | ||
addSubview(contentStackView) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
contentView.addSubview(contentStackView)
처럼 사용할때와 어떤 차이가 있을까요~?
만약 있다면 어떤 방법이 레이아웃 관리에 좀 더 도움이 될까요!? 🙋🏻
@GREENOVER
안녕하세요! 그린🌳
BMO🤖 & Serena🐷팀입니다!
박스오피스 프로젝트 II Step2 PR 보냅니다.
이번 리뷰도 잘 부탁드립니다!😆
구현한 내용
고민한 부분
🔥 setCollectionViewLayout을 활용하여 애니메이션 효과
setCollectionViewLayout
메서드를 활용하면 이런 부분을 해결할 수 있다는 것을 알게되어 이 부분을 수정하였습니다.🔥 UINavigationController - UIToolbar 사용
view
가 다음과 같은 구조가 되어야 한다고 생각했습니다.UINavigationController
문서를 보던 중built-in
toolbar
가 있다는 것을 알게되었습니다. 해당toolbar
는isToolbarHidden
프로퍼티를false
를 주는것만으로toolbar
를 노출시킬 수 있습니다.🔥 UserDefaults + Enum을 이용한 ViewMode 변경
UserDefaults
를 사용하기로 했습니다.Enum
타입을 추가했습니다.UserDefaults
에 값을 저장할 때Enum
타입을 넣을 수 없었기 때문에extension
으로 추가 작업을 해주었습니다.참고자료