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] Fix request desktop site action in new menu UI #27007

Merged
merged 1 commit into from
Dec 13, 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
Original file line number Diff line number Diff line change
Expand Up @@ -493,14 +493,15 @@ extension BrowserViewController {
func presentBrowserMenu(
from sourceView: UIView,
activities: [UIActivity],
tab: Tab?,
pageURL: URL?,
webView: WKWebView?
) {
var actions: [Action] = []
actions.append(vpnMenuAction)
actions.append(contentsOf: destinationMenuActions(for: pageURL))
actions.append(contentsOf: pageActions(for: pageURL, webView: webView))
let pageActivities: Set<Action> = Set(
var pageActivities: Set<Action> = Set(
activities
.compactMap { activity in
guard let id = (activity as? MenuActivity)?.id,
Expand All @@ -519,6 +520,27 @@ extension BrowserViewController {
}
}
)
if let tab,
let requestDesktopPageActivity = pageActivities.first(where: { $0.id == .requestDesktopSite })
{
// Remove the UIActivity version and replace it with a manual version.
// The request desktop activity is special in the sense that it is dynamic based on the
// current tab user agent, but we don't use rely on the UIActivity information to populate
// actions in the new menu UI, so this replaces it with how we would compose it manually
pageActivities.remove(requestDesktopPageActivity)
pageActivities.insert(
.init(
id: .requestDesktopSite,
title: tab.isDesktopSite ? Strings.appMenuViewMobileSiteTitleString : nil,
image: tab.isDesktopSite ? "leo.smartphone" : nil,
handler: { @MainActor [unowned self, weak tab] _ in
tab?.switchUserAgent()
self.dismiss(animated: true)
return .none
}
)
)
}
// Sets up empty actions for any page actions that weren't setup as UIActivity's
let remainingPageActivities: [Action] = Action.ID.allPageActivites
.subtracting(pageActivities.map(\.id))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,15 @@ extension BrowserViewController {
}

// Request Desktop Site Activity
let isDesktopSite = tab?.isDesktopSite ?? false
var requestDesktopSite = BasicMenuActivity.ActivityType.requestDesktopSite
if isDesktopSite {
requestDesktopSite.title = Strings.appMenuViewMobileSiteTitleString
requestDesktopSite.braveSystemImage = "leo.smartphone"
}
activities.append(
BasicMenuActivity(
activityType: tab?.isDesktopSite == true ? .requestMobileSite : .requestDesktopSite,
activityType: requestDesktopSite,
callback: {
tab?.switchUserAgent()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,7 @@ extension BrowserViewController: ToolbarDelegate {
presentBrowserMenu(
from: tabToolbar.menuButton,
activities: activities,
tab: tabManager.selectedTab,
pageURL: selectedTabURL,
webView: tabManager.selectedTab?.webView
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,8 @@ extension BasicMenuActivity.ActivityType {
title: Strings.addToFavorites,
braveSystemImage: "leo.widget.generic"
)
static let requestMobileSite: Self = .init(
id: "RequestMobileSite",
title: Strings.appMenuViewMobileSiteTitleString,
braveSystemImage: "leo.smartphone"
)
static let requestDesktopSite: Self = .init(
id: "RequestDesktopSite",
id: "ToggleUserAgent",
title: Strings.appMenuViewDesktopSiteTitleString,
braveSystemImage: "leo.monitor"
)
Expand Down
12 changes: 4 additions & 8 deletions ios/brave-ios/Sources/BrowserMenu/BrowserMenu.swift
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,8 @@ public struct BrowserMenu: View {
}
}
.modifier(MenuRowButtonStyleModifier())
.background(
Color(braveSystemName: .iosBrowserContainerHighlightIos),
in: .rect(cornerRadius: 14, style: .continuous)
)
.background(Color(braveSystemName: .iosBrowserContainerHighlightIos))
.clipShape(.rect(cornerRadius: 14, style: .continuous))
Comment on lines -135 to +136
Copy link
Collaborator Author

@kylehickinson kylehickinson Dec 13, 2024

Choose a reason for hiding this comment

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

Small UI bug I noticed that I snuck in, just ensures the button highlight is clipped to the background shape

.transition(.blurReplace())
}
Button {
Expand All @@ -144,10 +142,8 @@ public struct BrowserMenu: View {
Label(Strings.BrowserMenu.allSettingsButtonTitle, braveSystemImage: "leo.settings")
}
.modifier(MenuRowButtonStyleModifier())
.background(
Color(braveSystemName: .iosBrowserContainerHighlightIos),
in: .rect(cornerRadius: 14, style: .continuous)
)
.background(Color(braveSystemName: .iosBrowserContainerHighlightIos))
.clipShape(.rect(cornerRadius: 14, style: .continuous))
}
.padding()
}
Expand Down
Loading