Skip to content

Commit

Permalink
[iOS] Cleanup popup menus
Browse files Browse the repository at this point in the history
This CL cleans up the popup menus now that SF Symbols are launched.
The SF Symbols launch replaced the custom popup menus displayed from
the toolbars by system menus.
The popup menus are now only used on iOS 14 for the tools menu.

Bug: 1425175
Change-Id: Ic2e24b28547e0f6c70b13d0e0332fed781251d10
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4364081
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Ewann Pellé <ewannpv@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1121319}
  • Loading branch information
Gauthier Ambard authored and Chromium LUCI CQ committed Mar 23, 2023
1 parent 059d70a commit 2fc3d33
Show file tree
Hide file tree
Showing 65 changed files with 323 additions and 1,707 deletions.
2 changes: 0 additions & 2 deletions ios/chrome/browser/metrics/first_user_action_recorder.cc
Expand Up @@ -41,7 +41,6 @@ const char* kIgnoredActions[] = {
"MobileFirstUserAction_NewTask",
"MobileMenuCloseAllTabs",
"MobileMenuCloseAllIncognitoTabs",
"MobileMenuCloseTab",
"MobileNewTabOpened",
"MobileTabClosed",
"MobileTabStripCloseTab",
Expand All @@ -65,7 +64,6 @@ const char* kNewTaskActions[] = {
"MobileMenuNewIncognitoTab",
"MobileMenuNewTab",
"MobileMenuRecentTabs",
"MobileMenuVoiceSearch",
"MobileBookmarkManagerEntryOpened",
"MobileRecentTabManagerTabFromOtherDeviceOpened",
"MobileNTPMostVisited",
Expand Down
Expand Up @@ -156,19 +156,8 @@ - (void)testTabGridButtonLongPressMenuWhenIncognitoAvailable {
[[EarlGrey selectElementWithMatcher:TabGridButton()]
performAction:grey_longPress()];

if ([ChromeEarlGrey isSFSymbolEnabled]) {
AssertItemEnabled(IDS_IOS_TOOLS_MENU_NEW_TAB, /*enabled=*/true);
AssertItemEnabled(IDS_IOS_TOOLS_MENU_NEW_INCOGNITO_TAB, /*enabled=*/true);
} else {
AssertItemEnabledState(
grey_accessibilityID(kToolsMenuNewTabId),
grey_accessibilityID(kPopupMenuTabGridMenuTableViewId),
/*enabled=*/YES);
AssertItemEnabledState(
grey_accessibilityID(kToolsMenuNewIncognitoTabId),
grey_accessibilityID(kPopupMenuTabGridMenuTableViewId),
/*enabled=*/YES);
}
AssertItemEnabled(IDS_IOS_TOOLS_MENU_NEW_TAB, /*enabled=*/true);
AssertItemEnabled(IDS_IOS_TOOLS_MENU_NEW_INCOGNITO_TAB, /*enabled=*/true);
}

// When the IncognitoModeAvailability policy is set to disabled, the "New
Expand All @@ -180,19 +169,8 @@ - (void)testTabGridButtonLongPressMenuWhenIncognitoDisabled {
[[EarlGrey selectElementWithMatcher:TabGridButton()]
performAction:grey_longPress()];

if ([ChromeEarlGrey isSFSymbolEnabled]) {
AssertItemEnabled(IDS_IOS_TOOLS_MENU_NEW_TAB, /*enabled=*/true);
AssertItemEnabled(IDS_IOS_TOOLS_MENU_NEW_INCOGNITO_TAB, /*enabled=*/false);
} else {
AssertItemEnabledState(
grey_accessibilityID(kToolsMenuNewTabId),
grey_accessibilityID(kPopupMenuTabGridMenuTableViewId),
/*enabled=*/YES);
AssertItemEnabledState(
grey_accessibilityID(kToolsMenuNewIncognitoTabId),
grey_accessibilityID(kPopupMenuTabGridMenuTableViewId),
/*enabled=*/NO);
}
AssertItemEnabled(IDS_IOS_TOOLS_MENU_NEW_TAB, /*enabled=*/true);
AssertItemEnabled(IDS_IOS_TOOLS_MENU_NEW_INCOGNITO_TAB, /*enabled=*/false);
}

// When the IncognitoModeAvailability policy is set to forced, the "New Tab"
Expand All @@ -204,17 +182,8 @@ - (void)testTabGridButtonLongPressMenuWhenIncognitoOnly {
[[EarlGrey selectElementWithMatcher:TabGridButton()]
performAction:grey_longPress()];

if ([ChromeEarlGrey isSFSymbolEnabled]) {
AssertItemEnabled(IDS_IOS_TOOLS_MENU_NEW_TAB, /*enabled=*/false);
AssertItemEnabled(IDS_IOS_TOOLS_MENU_NEW_INCOGNITO_TAB, /*enabled=*/true);
} else {
AssertItemEnabledState(
grey_accessibilityID(kToolsMenuNewTabId),
grey_accessibilityID(kPopupMenuTabGridMenuTableViewId), NO);
AssertItemEnabledState(
grey_accessibilityID(kToolsMenuNewIncognitoTabId),
grey_accessibilityID(kPopupMenuTabGridMenuTableViewId), YES);
}
AssertItemEnabled(IDS_IOS_TOOLS_MENU_NEW_TAB, /*enabled=*/false);
AssertItemEnabled(IDS_IOS_TOOLS_MENU_NEW_INCOGNITO_TAB, /*enabled=*/true);
}

// TODO(crbug.com/1165655): Add test to new tab long-press menu.
Expand Down
4 changes: 2 additions & 2 deletions ios/chrome/browser/shared/public/commands/browser_commands.h
Expand Up @@ -20,8 +20,8 @@
// TODO(crbug.com/1272540): Remove this command.
- (void)addToReadingList:(ReadingListAddCommand*)command;

// Prepares the browser to display a popup menu.
- (void)prepareForPopupMenuPresentation:(PopupMenuCommandType)type;
// Prepares the browser to display the overflow menu.
- (void)prepareForOverflowMenuPresentation;

@end

Expand Down
14 changes: 0 additions & 14 deletions ios/chrome/browser/shared/public/commands/popup_menu_commands.h
Expand Up @@ -11,25 +11,11 @@ namespace web {
class WebState;
}

// Type of a popup menu command.
typedef NS_ENUM(NSInteger, PopupMenuCommandType) {
PopupMenuCommandTypeToolsMenu,
PopupMenuCommandTypeDefault,
};

// Commands for the popup menu.
@protocol PopupMenuCommands

// Shows the navigation history popup containing the back history.
- (void)showNavigationHistoryBackPopupMenu;
// Shows the navigation history popup containing the forward history.
- (void)showNavigationHistoryForwardPopupMenu;
// Shows the tools menu.
- (void)showToolsMenuPopup;
// Shows the popup for the tab grid button.
- (void)showTabGridButtonPopup;
// Shows the popup for the new tab button.
- (void)showNewTabButtonPopup;
// Dismisses the currently presented popup.
- (void)dismissPopupMenuAnimated:(BOOL)animated;
// Shows a snackbar that allows the user to UNDO its pin/unpin action.
Expand Down
3 changes: 0 additions & 3 deletions ios/chrome/browser/shared/ui/util/BUILD.gn
Expand Up @@ -17,8 +17,6 @@ source_set("util") {
"attributed_string_util.mm",
"dynamic_type_util.h",
"dynamic_type_util.mm",
"force_touch_long_press_gesture_recognizer.h",
"force_touch_long_press_gesture_recognizer.mm",
"keyboard_observer_helper.h",
"keyboard_observer_helper.mm",
"layout_guide_names.h",
Expand Down Expand Up @@ -115,7 +113,6 @@ source_set("unit_tests") {
configs += [ "//build/config/compiler:enable_arc" ]
testonly = true
sources = [
"force_touch_long_press_gesture_recognizer_unittest.mm",
"frame_layout_guide_unittest.mm",
"layout_guide_center_unittest.mm",
"named_guide_unittest.mm",
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

4 changes: 0 additions & 4 deletions ios/chrome/browser/ui/browser_view/browser_coordinator.mm
Expand Up @@ -770,17 +770,13 @@ - (void)createViewControllerDependencies {
// behavior but helps command handler setup below.
[self.popupMenuCoordinator start];

_primaryToolbarCoordinator.longPressDelegate = self.popupMenuCoordinator;
_secondaryToolbarCoordinator.longPressDelegate = self.popupMenuCoordinator;

if (ui::GetDeviceFormFactor() == ui::DEVICE_FORM_FACTOR_TABLET) {
if (base::FeatureList::IsEnabled(kModernTabStrip)) {
_tabStripCoordinator =
[[TabStripCoordinator alloc] initWithBrowser:self.browser];
} else {
_legacyTabStripCoordinator =
[[TabStripLegacyCoordinator alloc] initWithBrowser:self.browser];
_legacyTabStripCoordinator.longPressDelegate = self.popupMenuCoordinator;
_legacyTabStripCoordinator.animationWaitDuration =
kLegacyFullscreenControllerToolbarAnimationDuration.InSecondsF();

Expand Down
6 changes: 2 additions & 4 deletions ios/chrome/browser/ui/browser_view/browser_view_controller.mm
Expand Up @@ -2979,7 +2979,7 @@ - (void)addToReadingList:(ReadingListAddCommand*)command {
[self addURLsToReadingList:command.URLs];
}

- (void)prepareForPopupMenuPresentation:(PopupMenuCommandType)type {
- (void)prepareForOverflowMenuPresentation {
DCHECK(self.browserState);
DCHECK(self.visible || self.dismissingModal);

Expand All @@ -2993,9 +2993,7 @@ - (void)prepareForPopupMenuPresentation:(PopupMenuCommandType)type {
// Allow the non-modal promo scheduler to close the promo.
[self.nonModalPromoScheduler logPopupMenuEntered];

if (type == PopupMenuCommandTypeToolsMenu) {
[_bubblePresenter toolsMenuDisplayed];
}
[_bubblePresenter toolsMenuDisplayed];
}

#pragma mark - TabConsumer (Public)
Expand Down
5 changes: 0 additions & 5 deletions ios/chrome/browser/ui/popup_menu/BUILD.gn
Expand Up @@ -19,7 +19,6 @@ source_set("popup_menu") {
":constants",
"resources:popup_menu_add_bookmark",
"resources:popup_menu_bookmarks",
"resources:popup_menu_close_tab",
"resources:popup_menu_downloads",
"resources:popup_menu_edit_bookmark",
"resources:popup_menu_enterprise_icon",
Expand All @@ -30,24 +29,20 @@ source_set("popup_menu") {
"resources:popup_menu_new_incognito_tab",
"resources:popup_menu_new_tab",
"resources:popup_menu_new_window",
"resources:popup_menu_paste_and_go",
"resources:popup_menu_price_notifications",
"resources:popup_menu_qr_scanner",
"resources:popup_menu_read_later",
"resources:popup_menu_reading_list",
"resources:popup_menu_recent_tabs",
"resources:popup_menu_reload",
"resources:popup_menu_report_an_issue",
"resources:popup_menu_request_desktop_site",
"resources:popup_menu_request_mobile_site",
"resources:popup_menu_search",
"resources:popup_menu_settings",
"resources:popup_menu_site_information",
"resources:popup_menu_stop",
"resources:popup_menu_text_zoom",
"resources:popup_menu_translate",
"resources:popup_menu_unfollow",
"resources:popup_menu_voice_search",
"//base",
"//components/bookmarks/browser",
"//components/bookmarks/common",
Expand Down
2 changes: 0 additions & 2 deletions ios/chrome/browser/ui/popup_menu/cells/BUILD.gn
Expand Up @@ -5,8 +5,6 @@
source_set("cells") {
configs += [ "//build/config/compiler:enable_arc" ]
sources = [
"popup_menu_navigation_item.h",
"popup_menu_navigation_item.mm",
"popup_menu_text_item.h",
"popup_menu_text_item.mm",
"popup_menu_tools_item.h",
Expand Down

0 comments on commit 2fc3d33

Please sign in to comment.