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(iOS 18.1): Set explicit arrow edge position for popovers #1610

Merged
merged 1 commit into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion Mail/Components/ActionsPanelButton.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ struct ActionsPanelButton<Content: View>: View {
let messages: [Message]
let originFolder: Folder?
let panelSource: ActionOrigin.FloatingPanelSource
var popoverArrowEdge: Edge = .top
@ViewBuilder var label: () -> Content

var body: some View {
Expand All @@ -35,7 +36,12 @@ struct ActionsPanelButton<Content: View>: View {
} label: {
label()
}
.actionsPanel(messages: $actionMessages, originFolder: originFolder, panelSource: panelSource) { action in
.actionsPanel(
messages: $actionMessages,
originFolder: originFolder,
panelSource: panelSource,
popoverArrowEdge: popoverArrowEdge
) { action in
if action == .markAsUnread {
dismiss()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,14 @@ extension View {
messages: Binding<[Message]?>,
originFolder: Folder?,
panelSource: ActionOrigin.FloatingPanelSource,
popoverArrowEdge: Edge,
completionHandler: ((Action) -> Void)? = nil
) -> some View {
return modifier(ActionsPanelViewModifier(
messages: messages,
originFolder: originFolder,
panelSource: panelSource,
popoverArrowEdge: popoverArrowEdge,
completionHandler: completionHandler
))
}
Expand All @@ -55,7 +57,7 @@ struct ActionsPanelViewModifier: ViewModifier {
@Binding var messages: [Message]?
let originFolder: Folder?
let panelSource: ActionOrigin.FloatingPanelSource

var popoverArrowEdge: Edge
var completionHandler: ((Action) -> Void)?

private var origin: ActionOrigin {
Expand All @@ -74,7 +76,7 @@ struct ActionsPanelViewModifier: ViewModifier {
}

func body(content: Content) -> some View {
content.adaptivePanel(item: $messages) { messages in
content.adaptivePanel(item: $messages, popoverArrowEdge: popoverArrowEdge) { messages in
ActionsView(
user: currentUser.value,
target: messages,
Expand Down
3 changes: 2 additions & 1 deletion Mail/Views/Search/SearchModifiers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ struct SearchToolbar: ViewModifier {
.actionsPanel(
messages: $multipleSelectedMessages,
originFolder: viewModel.frozenSearchFolder,
panelSource: .threadList
panelSource: .threadList,
popoverArrowEdge: .leading
) { action in
viewModel.refreshSearchIfNeeded(action: action)
multipleSelectionViewModel.disable()
Expand Down
3 changes: 2 additions & 1 deletion Mail/Views/Thread List/ThreadListModifiers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ struct ThreadListToolbar: ViewModifier {
.actionsPanel(
messages: $multipleSelectedMessages,
originFolder: viewModel.frozenFolder,
panelSource: .threadList
panelSource: .threadList,
popoverArrowEdge: .bottom
) { action in
guard action != .openMovePanel else { return }
multipleSelectionViewModel.disable()
Expand Down
7 changes: 6 additions & 1 deletion Mail/Views/Thread List/ThreadListSwipeAction.swift
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,12 @@ struct ThreadListSwipeActions: ViewModifier {
edgeActions([swipeFullTrailing, swipeTrailing])
}
}
.actionsPanel(messages: $actionPanelMessages, originFolder: thread.folder, panelSource: .threadList) { action in
.actionsPanel(
messages: $actionPanelMessages,
originFolder: thread.folder,
panelSource: .threadList,
popoverArrowEdge: .leading
) { action in
viewModel.refreshSearchIfNeeded(action: action)
}
.sheet(item: $messagesToMove) { messages in
Expand Down
9 changes: 7 additions & 2 deletions Mail/Views/Thread/WebView/ThreadViewToolbarModifier.swift
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ struct ThreadViewToolbarModifier: ViewModifier {
ToolbarButton(text: action.title, icon: action.icon) {
didTap(action: action)
}
.adaptivePanel(item: $replyOrReplyAllMessage) { message in
.adaptivePanel(item: $replyOrReplyAllMessage, popoverArrowEdge: .bottom) { message in
ReplyActionsView(message: message)
}
} else {
Expand All @@ -82,7 +82,12 @@ struct ThreadViewToolbarModifier: ViewModifier {
}
}
}
ActionsPanelButton(messages: frozenMessages, originFolder: frozenFolder, panelSource: .messageList) {
ActionsPanelButton(
messages: frozenMessages,
originFolder: frozenFolder,
panelSource: .messageList,
popoverArrowEdge: .bottom
) {
ToolbarButtonLabel(text: MailResourcesStrings.Localizable.buttonMore,
icon: MailResourcesAsset.plusActions.swiftUIImage)
}
Expand Down
63 changes: 52 additions & 11 deletions MailCoreUI/Utils/AdaptivePanelViewModifier.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,26 +21,26 @@ import MailResources
import SwiftUI

public extension View {
func adaptivePanel<Item: Identifiable, Content: View>(item: Binding<Item?>,
@ViewBuilder content: @escaping (Item) -> Content) -> some View {
return modifier(AdaptivePanelViewModifier(item: item, panelContent: content))
func adaptivePanel<Item: Identifiable, Content: View>(
item: Binding<Item?>,
popoverArrowEdge: Edge = .top,
@ViewBuilder content: @escaping (Item) -> Content
) -> some View {
return modifier(AdaptivePanelViewModifier(item: item, popoverArrowEdge: popoverArrowEdge, panelContent: content))
}
}

public struct AdaptivePanelViewModifier<Item: Identifiable, PanelContent: View>: ViewModifier {
struct AdaptivePanelViewModifier<Item: Identifiable, PanelContent: View>: ViewModifier {
@Environment(\.isCompactWindow) private var isCompactWindow

@Binding var item: Item?
@ViewBuilder let panelContent: (Item) -> PanelContent

public init(item: Binding<Item?>, panelContent: @escaping (Item) -> PanelContent) {
_item = item
self.panelContent = panelContent
}
var popoverArrowEdge: Edge
@ViewBuilder let panelContent: (Item) -> PanelContent

public func body(content: Content) -> some View {
func body(content: Content) -> some View {
content
.popover(item: $item) { item in
.workaroundPopover(item: $item, arrowEdge: popoverArrowEdge) { item in
if isCompactWindow {
if #available(iOS 16.0, *) {
panelContent(item).modifier(SelfSizingPanelViewModifier())
Expand All @@ -59,3 +59,44 @@ public struct AdaptivePanelViewModifier<Item: Identifiable, PanelContent: View>:
}
}
}

// MARK: - WorkaroundPopover

private extension View {
func workaroundPopover<Item: Identifiable, Content: View>(
item: Binding<Item?>,
arrowEdge: Edge,
@ViewBuilder content: @escaping (Item) -> Content
) -> some View {
modifier(WorkaroundPopover(item: item, arrowEdge: arrowEdge, panelContent: content))
}
}

// FIXME: Remove this workaround when the Release Candidate of Xcode 16.2 is release
/// iOS 18.1 introduces an issue with the popover (135231043) fixed by iOS 18.2 (the bug is also visible on iOS 18.0)
///
/// In this specific version of iOS, the `popover` type signature has been changed. The `arrowEdge: Edge?`
/// parameter is no longer optional with the default value of nil. The default value is now `Edge.top`
/// which causes the popover to be hidden in some cases.
///
/// Therefore for iOS 18.1 and later versions (while we build with Xcode 16.1) we have to provide a value for `arrowEdge`.
private struct WorkaroundPopover<Item: Identifiable, PanelContent: View>: ViewModifier {
@Binding var item: Item?

let arrowEdge: Edge
@ViewBuilder let panelContent: (Item) -> PanelContent

func body(content: Content) -> some View {
if #available(iOS 18.0, *) {
content
.popover(item: $item, arrowEdge: arrowEdge) { item in
panelContent(item)
}
} else {
content
.popover(item: $item) { item in
panelContent(item)
}
}
}
}
Loading