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

[iOS] Improve new menu UI on iPad & expand menu when showing all actions #27137

Merged
merged 1 commit into from
Jan 7, 2025
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
44 changes: 39 additions & 5 deletions ios/brave-ios/Sources/BrowserMenu/BrowserMenu.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@ import SwiftUI
public struct BrowserMenu: View {
@ObservedObject var model: BrowserMenuModel
var handlePresentation: (BrowserMenuPresentation) -> Void
var onShowAllActions: (() -> Void)?

@State private var isEditMenuPresented = false

/// Whether or not we may be viewing on a device that doesn't have as much horizontal space
/// available (such as when Display Zoom is enabled)
@State private var isHorizontalSpaceRestricted: Bool = false
@State private var activeActionHandlers: [Action.ID: Task<Void, Never>] = [:]
@State private var isAdditionalActionsVisible: Bool = false

@Environment(\.dynamicTypeSize) private var dynamicTypeSize

Expand Down Expand Up @@ -112,6 +114,7 @@ public struct BrowserMenu: View {
ActionsList(
actions: listedActions,
additionalActions: $model.hiddenActions,
isAdditionalActionsVisible: $isAdditionalActionsVisible,
handler: { $action in
handleAction($action)
}
Expand Down Expand Up @@ -161,6 +164,11 @@ public struct BrowserMenu: View {
isHorizontalSpaceRestricted = newValue
}
)
.onChange(of: isAdditionalActionsVisible) { newValue in
if newValue {
onShowAllActions?()
}
}
}
}

Expand Down Expand Up @@ -262,7 +270,15 @@ private struct QuickActionsView: View {
.frame(width: iconFrameSize, height: iconFrameSize)
.padding(.vertical, 12)
.font(.system(size: iconFontSize))
.foregroundStyle(isEnabled ? Color(braveSystemName: .iconDefault) : .secondary)
.foregroundStyle(
isEnabled
? Color(braveSystemName: .iconDefault)
// There's a SwiftUI bug on iPads where the items don't animate correctly due to using
// material hierarchical foreground styles so for this we fall back to the design
// system colors. This is also applied to the menu row label styles.
: (UIDevice.current.userInterfaceIdiom == .pad
? Color(braveSystemName: .iconSecondary) : .secondary)
)
.frame(maxWidth: .infinity)
.background {
Color(braveSystemName: .iosBrowserContainerHighlightIos)
Expand All @@ -283,7 +299,12 @@ private struct QuickActionsView: View {
configuration.title
.font(.caption2)
.lineLimit(2)
.foregroundStyle(isEnabled ? Color(braveSystemName: .textPrimary) : .secondary)
.foregroundStyle(
isEnabled
? Color(braveSystemName: .textPrimary)
: (UIDevice.current.userInterfaceIdiom == .pad
? Color(braveSystemName: .textSecondary) : .secondary)
)
.multilineTextAlignment(.center)
}
.opacity(isEnabled ? 1 : 0.7)
Expand Down Expand Up @@ -332,15 +353,18 @@ private struct ActionButton: View {
private struct ActionsList: View {
@Binding var actions: [Action]
@Binding var additionalActions: [Action]
@Binding var isAdditionalActionsVisible: Bool
var handler: (Binding<Action>) -> Void

init(
actions: Binding<[Action]>,
additionalActions: Binding<[Action]>,
isAdditionalActionsVisible: Binding<Bool>,
handler: @escaping (Binding<Action>) -> Void
) {
self._actions = actions
self._additionalActions = additionalActions
self._isAdditionalActionsVisible = isAdditionalActionsVisible
self.handler = handler
}

Expand All @@ -350,12 +374,12 @@ private struct ActionsList: View {
) {
self._actions = .constant([action])
self._additionalActions = .constant([])
self._isAdditionalActionsVisible = .constant(false)
self.handler = { action in
handler(action.wrappedValue)
}
}

@State private var isAdditionalActionsVisible: Bool = false
@Environment(\.pixelLength) private var pixelLength
@ScaledMetric private var badgeRadius = 8

Expand Down Expand Up @@ -497,10 +521,20 @@ private struct MenuRowButtonStyleModifier: ViewModifier {
configuration.icon
.frame(width: iconFrameSize, height: iconFrameSize)
.font(.system(size: iconFontSize))
.foregroundStyle(isEnabled ? Color(braveSystemName: .iconDefault) : .secondary)
.foregroundStyle(
isEnabled
? Color(braveSystemName: .iconDefault)
: (UIDevice.current.userInterfaceIdiom == .pad
? Color(braveSystemName: .iconSecondary) : .secondary)
)
configuration.title
.font(.body)
.foregroundStyle(isEnabled ? Color(braveSystemName: .textPrimary) : .secondary)
.foregroundStyle(
isEnabled
? Color(braveSystemName: .textPrimary)
: (UIDevice.current.userInterfaceIdiom == .pad
? Color(braveSystemName: .textSecondary) : .secondary)
)
}
.padding(.vertical, 12)
.opacity(isEnabled ? 1 : 0.7)
Expand Down
14 changes: 14 additions & 0 deletions ios/brave-ios/Sources/BrowserMenu/BrowserMenuController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,17 @@ public class BrowserMenuController: UIHostingController<BrowserMenu> {
handlePresentation: handlePresentation
)
)
rootView.onShowAllActions = { [weak self] in
guard let self else { return }
if let controller = sheetPresentationController
?? popoverPresentationController?.adaptiveSheetPresentationController
{
controller.animateChanges {
controller.selectedDetentIdentifier = .large
}
}
preferredContentSize.height = view.intrinsicContentSize.height
}
}

required init?(coder: NSCoder) {
Expand All @@ -37,6 +48,7 @@ public class BrowserMenuController: UIHostingController<BrowserMenu> {
super.viewDidLoad()

view.backgroundColor = .clear
popoverPresentationController?.backgroundColor = .clear
}

public override func viewIsAppearing(_ animated: Bool) {
Expand All @@ -53,5 +65,7 @@ public class BrowserMenuController: UIHostingController<BrowserMenu> {
]
controller.prefersGrabberVisible = true
}

preferredContentSize = CGSize(width: 375, height: size.height)
}
}
Loading