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.

Sign up for GitHub

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

Jump to bottom

merge :: Dms Bottom Sheet Modifier #87

Merged
merged 7 commits into from
Oct 25, 2022
Merged

merge :: Dms Bottom Sheet Modifier #87

merged 7 commits into from
Oct 25, 2022

Conversation

baekteun
Copy link
Member

@baekteun baekteun commented Oct 25, 2022 •
edited
Loading

개요

Simulator.Screen.Recording.-.iPhone.14.Pro.-.2022-10-25.at.11.51.36.mp4

작업사항

  • DmsBottomSheet 작업

사용방법

VStack {
}
.dmsBottomSheet(isShowing: $isShowing) {
    Text("ASDAF")
        .padding(42)
}

@baekteun baekteun added the ✨feat 새로운 기능을 추가 할 경우 label Oct 25, 2022
@baekteun baekteun self-assigned this Oct 25, 2022
@baekteun baekteun linked an issue Oct 25, 2022 that may be closed by this pull request
BottomSheet modifier #85
Closed
@baekteun baekteun requested a review from sian7563 as a code owner October 25, 2022 02:52
kimdaehee0824
kimdaehee0824 reviewed Oct 25, 2022
View reviewed changes
Copy link
Member

@kimdaehee0824 kimdaehee0824 left a comment

Choose a reason for hiding this comment

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

궁금해서 물어보는건데 iOS의 기본적인 Bottom Sheet를 사용하지 않은 이유가 있을까요?

kimdaehee0824
kimdaehee0824 reviewed Oct 25, 2022
View reviewed changes
Projects/UsertInterfaces/DesignSystem/Sources/BottomSheet/DmsBottomSheet.swift Outdated
Comment on lines 27 to 33
init(
isShowing: Binding<Bool>,
content: @escaping () -> T
) {
_isShowing = isShowing
self.content = content
}
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
Member Author

Choose a reason for hiding this comment

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

content에 따라 동적으로 크키가 바뀌게 해놓았는데 height는 필요할까 싶어서 안받아놓긴 했습니다

@baekteun
Copy link
Member Author

궁금해서 물어보는건데 iOS의 기본적인 Bottom Sheet를 사용하지 않은 이유가 있을까요?

  1. 최신 API인 presentationDetents는 iOS 16 부터 지원이 돼서 제외해놓았습니다.
  2. UISheetPresentationController를 UIViewControllerRepresentable로 래핑하여 사용한다면 크기를 .medium(), large() 사이즈로만 조절 가능하기에 컨텐츠 표시를 의도대로 할 수 없을 수도 있기 때문입니다.

@baekteun baekteun merged commit 6e1da38 into develop Oct 25, 2022
@baekteun baekteun deleted the 85-bottomsheet-modifier branch October 25, 2022 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sian7563 sian7563 sian7563 approved these changes

@kimdaehee0824 kimdaehee0824 kimdaehee0824 approved these changes

Assignees

@baekteun baekteun

Labels
✨feat 새로운 기능을 추가 할 경우
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BottomSheet modifier
3 participants
@baekteun @kimdaehee0824 @sian7563