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

Fix Example + add bottom sheet to 2.0.0 #23

Open
wants to merge 3 commits into
base: release/2.0.0
Choose a base branch
from

Conversation

HaidenTenno
Copy link
Contributor

No description provided.

@HaidenTenno HaidenTenno changed the title Fix Example + add bottom sheet Fix Example + add bottom sheet to 2.0.0 Mar 1, 2024
@onl1ner onl1ner added this to the 2.0.0 milestone Mar 4, 2024
@onl1ner onl1ner linked an issue Mar 4, 2024 that may be closed by this pull request
@HaidenTenno HaidenTenno requested a review from onl1ner March 5, 2024 10:37
Copy link
Member

@onl1ner onl1ner left a comment

Choose a reason for hiding this comment

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

Глянул свежим взглядом, появилось еще пару предложений

Comment on lines +8 to +16
public struct NavigationStore {
public var isAlertShow: Bool = false
public var hasOverlay: Bool = false

public init(isAlertShow: Bool = false, hasOverlay: Bool = false) {
self.isAlertShow = isAlertShow
self.hasOverlay = hasOverlay
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Я думаю будет хорошей идеей подписать Store под ObservableObject, чтобы юзеры могли подписываться под обновления. Быстренько набросал то как я вижу эти изменения:

Suggested change
public struct NavigationStore {
public var isAlertShow: Bool = false
public var hasOverlay: Bool = false
public init(isAlertShow: Bool = false, hasOverlay: Bool = false) {
self.isAlertShow = isAlertShow
self.hasOverlay = hasOverlay
}
}
public final class NavigationStore: ObservableObject {
@Published public var isAlertPresented: Bool = false
@Published public var isModalPresented: Bool = false
}

Comment on lines +10 to +35
class BSBuilder {

static func buildBottomSheet(
coordinator: Coordinator,
screen: any NavigationScreen,
size: BSSize
) {
switch size {
case .auto:
coordinator.bottomSheetTransitioningDelegate.sheetSize = .auto
screen.transitioningDelegate = coordinator.bottomSheetTransitioningDelegate
screen.modalPresentationStyle = .custom
case .fixed(let height):
coordinator.bottomSheetTransitioningDelegate.sheetSize = .fixed(height)
screen.transitioningDelegate = coordinator.bottomSheetTransitioningDelegate
screen.modalPresentationStyle = .custom
case .halfScreen:
coordinator.bottomSheetTransitioningDelegate.sheetSize = .halfScreen
screen.transitioningDelegate = coordinator.bottomSheetTransitioningDelegate
screen.modalPresentationStyle = .custom
case .fullScreen:
screen.modalPresentationStyle = .formSheet
}
coordinator.store.hasOverlay = true
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Может тогда лучше будем хранить делегат тут, чтобы не прокидывать координатор сюда, а то как-то слишком неочевидно получается. Давай еще переименуем функцию просто в build(screen:size:), а то c окончанием bottomSheet будто получается тавтология. Плюсом сюда думаю будет правильней возвращать уже сконфигурированный экран, чтобы не нарушать семантику, т.к. мы используем слово build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

Refactor BottomSheet presentation
2 participants