Skip to content

Commit

Permalink
[User Education] Enable highlighted menu item metadata in feature promos
Browse files Browse the repository at this point in the history
Add `.SetHighlightedMenuItem()` to FeaturePromoSpecification to allow
Promos to continue into menus where menu items will be highlighted.

This data is read in the AppMenuModel to continue a promo into the menu.

Update all IPH that currently anchor to the BrowserAppMenuButton when
SidePanelPinning is enabled.

Bug: 1493027
Change-Id: I79dc1019b3381383ff3673add157f9c008726d22
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4942995
Reviewed-by: Dana Fried <dfried@chromium.org>
Commit-Queue: Mickey Burks <mickeyburks@chromium.org>
Reviewed-by: Eshwar Stalin <estalin@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Caroline Rising <corising@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1212247}
  • Loading branch information
Mickey Burks authored and Chromium LUCI CQ committed Oct 19, 2023
1 parent e614468 commit b0aa105
Show file tree
Hide file tree
Showing 26 changed files with 489 additions and 98 deletions.
16 changes: 16 additions & 0 deletions chrome/app/generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -8329,6 +8329,9 @@ Keep your key file in a safe place. You will need it to create new versions of y
<message name="IDS_SIDE_PANEL_COMPANION_PROMO" desc="Text shown on promotional UI appearing next to the pinned companion button, which encourages users to try the feature.">
Get a page summary, related searches, and other useful info about this page
</message>
<message name="IDS_SIDE_PANEL_COMPANION_PROMO_PINNING" desc="Text shown on promotional UI appearing next to the pinned companion button, which encourages users to try the feature when Side Panel Pinning is enabled.">
You can search this page with Google to get additional useful info
</message>
<message name="IDS_SIDE_PANEL_COMPANION_PROMO_SCREEN_READER" is_accessibility_with_no_ui="true" desc="Screen reader announcement on the promotional UI appearing next to the pinned companion button, which excourages users to try the feature.">
To get a page summary, related searches, and other useful info about this page, select Google Search side panel button in the toolbar
</message>
Expand Down Expand Up @@ -8657,6 +8660,10 @@ Keep your key file in a safe place. You will need it to create new versions of y
desc="Text for the in-product-help bubble shown for the reading mode bubble.">
To show a simplified view of this page, open the side panel and select Reading mode
</message>
<message name="IDS_READING_MODE_SIDE_PANEL_PROMO_PINNING"
desc="Text for the in-product-help bubble shown for the reading mode bubble when Side Panel Pinning is enabled.">
To see a simplified view of this page, go to More Tools > Reading Mode
</message>

<!-- User note strings -->
<message name="IDS_USER_NOTE_TITLE" desc="Title of the User note feature, which gives users access to private user notes.">
Expand Down Expand Up @@ -9204,9 +9211,15 @@ Keep your key file in a safe place. You will need it to create new versions of y
<message name="IDS_POWER_BOOKMARKS_SIDE_PANEL_PROMO" desc="Text shown on promotional UI appearing next to the side panel button, which encourages users to view their bookmarks there.">
See all your bookmarks here
</message>
<message name="IDS_POWER_BOOKMARKS_SIDE_PANEL_PROMO_PINNING" desc="Text shown on promotional UI appearing next to the side panel button, which encourages users to view their bookmarks there when Side Panel Pinning is enabled.">
See all your bookmarks in Bookmarks and Lists
</message>
<message name="IDS_READING_LIST_DISCOVERY_PROMO" desc="Text shown on promotional UI appearing next to the reading list button">
When you're ready, find your reading list here
</message>
<message name="IDS_READING_LIST_DISCOVERY_PROMO_PINNING" desc="Text shown on promotional UI appearing next to the reading list button when Side Panel Pinning is enabled">
When you're ready, find your reading list in Bookmarks and Lists
</message>
<message name="IDS_READING_LIST_ENTRY_POINT_PROMO" desc="Text shown on promotional UI to encourage users to add a tab to their reading list">
To add this page to your reading list, click the Bookmark icon
</message>
Expand All @@ -9219,6 +9232,9 @@ Keep your key file in a safe place. You will need it to create new versions of y
<message name="IDS_READING_LIST_IN_SIDE_PANEL_PROMO" desc="Text shown on promotional UI appearing next to the side panel button">
Your reading list has moved to the new side panel. Try it here.
</message>
<message name="IDS_READING_LIST_IN_SIDE_PANEL_PROMO_PINNING" desc="Text shown on promotional UI appearing next to the side panel button when Side Panel Pinning is enabled">
Find your reading list and bookmarks in Bookmarks and Lists
</message>
<message name="IDS_TAB_GROUPS_NEW_GROUP_PROMO" desc="Text shown on promotional UI encouraging users to create a tab group in their tab strip">
Organize your tabs with tab groups
</message>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
239bcbe8a0b7d918c5b70e5e99daa4d9b6f76fed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
a4cb0e308f7445fdd59928fbc133089cdc7c530f
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
4312dcc992de7a92bb67f22817462cbabc24b7e4
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
9bc9c5231344398d4ca15ea9bd057afed5b4ddef
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fd16c3cc8f0dccb204b686a264f4d38e4cb98dd8
22 changes: 13 additions & 9 deletions chrome/browser/ui/toolbar/app_menu_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ DEFINE_CLASS_ELEMENT_IDENTIFIER_VALUE(AppMenuModel, kIncognitoMenuItem);
DEFINE_CLASS_ELEMENT_IDENTIFIER_VALUE(AppMenuModel,
kPasswordAndAutofillMenuItem);
DEFINE_CLASS_ELEMENT_IDENTIFIER_VALUE(AppMenuModel, kPasswordManagerMenuItem);
DEFINE_CLASS_ELEMENT_IDENTIFIER_VALUE(AppMenuModel, kShowSearchCompanion);
DEFINE_CLASS_ELEMENT_IDENTIFIER_VALUE(ToolsMenuModel, kPerformanceMenuItem);
DEFINE_CLASS_ELEMENT_IDENTIFIER_VALUE(ToolsMenuModel, kChromeLabsMenuItem);
DEFINE_CLASS_ELEMENT_IDENTIFIER_VALUE(ToolsMenuModel, kReadingModeMenuItem);
Expand Down Expand Up @@ -810,6 +811,11 @@ AppMenuModel::AppMenuModel(ui::AcceleratorProvider* provider,

AppMenuModel::~AppMenuModel() = default;

void AppMenuModel::SetHighlightedIdentifier(
ui::ElementIdentifier highlighted_menu_identifier) {
highlighted_menu_identifier_ = highlighted_menu_identifier;
}

void AppMenuModel::Init() {
Build();

Expand Down Expand Up @@ -1407,15 +1413,6 @@ bool AppMenuModel::IsCommandIdEnabled(int command_id) const {
}

bool AppMenuModel::IsCommandIdAlerted(int command_id) const {
if ((command_id == IDC_RECENT_TABS_MENU) ||
(command_id == AppMenuModel::kMinRecentTabsCommandId)) {
return alert_item_ == AlertMenuItem::kReopenTabs;
}

if (command_id == IDC_PERFORMANCE) {
return alert_item_ == AlertMenuItem::kPerformance;
}

if (command_id == IDC_VIEW_PASSWORDS ||
command_id == IDC_SHOW_PASSWORD_MANAGER) {
return alert_item_ == AlertMenuItem::kPasswordManager;
Expand All @@ -1424,6 +1421,10 @@ bool AppMenuModel::IsCommandIdAlerted(int command_id) const {
return false;
}

bool AppMenuModel::IsElementIdAlerted(ui::ElementIdentifier element_id) const {
return highlighted_menu_identifier_ == element_id;
}

bool AppMenuModel::GetAcceleratorForCommandId(
int command_id,
ui::Accelerator* accelerator) const {
Expand Down Expand Up @@ -1604,6 +1605,9 @@ void AppMenuModel::Build() {
#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
if (companion::IsCompanionFeatureEnabled()) {
AddItemWithStringId(IDC_SHOW_SEARCH_COMPANION, IDS_SHOW_SEARCH_COMPANION);
SetElementIdentifierAt(
GetIndexOfCommandId(IDC_SHOW_SEARCH_COMPANION).value(),
kShowSearchCompanion);
}
#endif
if (features::IsTabOrganization()) {
Expand Down
9 changes: 8 additions & 1 deletion chrome/browser/ui/toolbar/app_menu_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ enum AppMenuAction {
LIMIT_MENU_ACTION
};

enum class AlertMenuItem { kNone, kReopenTabs, kPerformance, kPasswordManager };
enum class AlertMenuItem { kNone, kPasswordManager };

// Function to record WrenchMenu.MenuAction histogram
void LogWrenchMenuAction(AppMenuAction action_id);
Expand Down Expand Up @@ -166,6 +166,7 @@ class AppMenuModel : public ui::SimpleMenuModel,
DECLARE_CLASS_ELEMENT_IDENTIFIER_VALUE(kIncognitoMenuItem);
DECLARE_CLASS_ELEMENT_IDENTIFIER_VALUE(kPasswordAndAutofillMenuItem);
DECLARE_CLASS_ELEMENT_IDENTIFIER_VALUE(kPasswordManagerMenuItem);
DECLARE_CLASS_ELEMENT_IDENTIFIER_VALUE(kShowSearchCompanion);

// Number of menus within the app menu with an arbitrarily high (variable)
// number of menu items. For example, the number of bookmarks menu items
Expand Down Expand Up @@ -198,6 +199,9 @@ class AppMenuModel : public ui::SimpleMenuModel,
// Runs Build() and registers observers.
void Init();

void SetHighlightedIdentifier(
ui::ElementIdentifier highlighted_menu_identifier);

// Overridden for ButtonMenuItemModel::Delegate:
bool DoesCommandIdDismissMenu(int command_id) const override;

Expand All @@ -206,6 +210,7 @@ class AppMenuModel : public ui::SimpleMenuModel,
bool IsCommandIdChecked(int command_id) const override;
bool IsCommandIdEnabled(int command_id) const override;
bool IsCommandIdAlerted(int command_id) const override;
bool IsElementIdAlerted(ui::ElementIdentifier element_id) const override;
bool GetAcceleratorForCommandId(int command_id,
ui::Accelerator* accelerator) const override;

Expand Down Expand Up @@ -276,6 +281,8 @@ class AppMenuModel : public ui::SimpleMenuModel,
PrefChangeRegistrar local_state_pref_change_registrar_;

const AlertMenuItem alert_item_;

ui::ElementIdentifier highlighted_menu_identifier_;
};

#endif // CHROME_BROWSER_UI_TOOLBAR_APP_MENU_MODEL_H_
12 changes: 11 additions & 1 deletion chrome/browser/ui/toolbar/bookmark_sub_menu_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,18 @@

DEFINE_CLASS_ELEMENT_IDENTIFIER_VALUE(BookmarkSubMenuModel,
kShowBookmarkBarMenuItem);
DEFINE_CLASS_ELEMENT_IDENTIFIER_VALUE(BookmarkSubMenuModel,
kShowBookmarkSidePanelItem);
DEFINE_CLASS_ELEMENT_IDENTIFIER_VALUE(BookmarkSubMenuModel,
kReadingListMenuItem);

// For views and cocoa, we have complex delegate systems to handle
// injecting the bookmarks to the bookmark submenu. This is done to support
// advanced interactions with the menu contents, like right click context menus.

BookmarkSubMenuModel::BookmarkSubMenuModel(
ui::SimpleMenuModel::Delegate* delegate, Browser* browser)
ui::SimpleMenuModel::Delegate* delegate,
Browser* browser)
: SimpleMenuModel(delegate) {
Build(browser);
}
Expand Down Expand Up @@ -53,6 +58,9 @@ void BookmarkSubMenuModel::Build(Browser* browser) {
if (features::IsChromeRefresh2023()) {
AddItemWithStringId(IDC_SHOW_BOOKMARK_SIDE_PANEL,
IDS_SHOW_BOOKMARK_SIDE_PANEL);
SetElementIdentifierAt(
GetIndexOfCommandId(IDC_SHOW_BOOKMARK_SIDE_PANEL).value(),
kShowBookmarkSidePanelItem);
}
AddItemWithStringId(IDC_SHOW_BOOKMARK_MANAGER, IDS_BOOKMARK_MANAGER);

Expand All @@ -69,6 +77,8 @@ void BookmarkSubMenuModel::Build(Browser* browser) {
IDC_READING_LIST_MENU, IDS_READING_LIST_MENU,
reading_list_sub_menu_model_.get(),
ui::ImageModel::FromVectorIcon(kReadLaterIcon));
SetElementIdentifierAt(GetIndexOfCommandId(IDC_READING_LIST_MENU).value(),
kReadingListMenuItem);

auto set_icon = [this](int command_id, const gfx::VectorIcon& vector_icon) {
auto index = GetIndexOfCommandId(command_id);
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/toolbar/bookmark_sub_menu_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ class ReadingListSubMenuModel;
class BookmarkSubMenuModel : public ui::SimpleMenuModel {
public:
DECLARE_CLASS_ELEMENT_IDENTIFIER_VALUE(kShowBookmarkBarMenuItem);
DECLARE_CLASS_ELEMENT_IDENTIFIER_VALUE(kShowBookmarkSidePanelItem);
DECLARE_CLASS_ELEMENT_IDENTIFIER_VALUE(kReadingListMenuItem);

BookmarkSubMenuModel(ui::SimpleMenuModel::Delegate* delegate,
Browser* browser);
Expand Down
6 changes: 6 additions & 0 deletions chrome/browser/ui/toolbar/reading_list_sub_menu_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
#include "chrome/app/vector_icons/vector_icons.h"
#include "chrome/grit/generated_resources.h"

DEFINE_CLASS_ELEMENT_IDENTIFIER_VALUE(ReadingListSubMenuModel,
kReadingListMenuShowUI);

ReadingListSubMenuModel::ReadingListSubMenuModel(
ui::SimpleMenuModel::Delegate* delegate)
: SimpleMenuModel(delegate) {
Expand All @@ -17,6 +20,9 @@ ReadingListSubMenuModel::ReadingListSubMenuModel(
AddItemWithStringIdAndIcon(IDC_READING_LIST_MENU_SHOW_UI,
IDS_READING_LIST_MENU_SHOW_UI,
ui::ImageModel::FromVectorIcon(kReadLaterIcon));
SetElementIdentifierAt(
GetIndexOfCommandId(IDC_READING_LIST_MENU_SHOW_UI).value(),
kReadingListMenuShowUI);
}

ReadingListSubMenuModel::~ReadingListSubMenuModel() = default;
2 changes: 2 additions & 0 deletions chrome/browser/ui/toolbar/reading_list_sub_menu_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
// includes "Add tab to Reading List" and "Show Reading List" entries.
class ReadingListSubMenuModel : public ui::SimpleMenuModel {
public:
DECLARE_CLASS_ELEMENT_IDENTIFIER_VALUE(kReadingListMenuShowUI);

explicit ReadingListSubMenuModel(ui::SimpleMenuModel::Delegate* delegate);

ReadingListSubMenuModel(const ReadingListSubMenuModel&) = delete;
Expand Down
23 changes: 20 additions & 3 deletions chrome/browser/ui/views/frame/app_menu_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "base/observer_list.h"
#include "chrome/browser/ui/browser_element_identifiers.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/toolbar/app_menu_model.h"
#include "chrome/browser/ui/views/chrome_typography.h"
#include "chrome/browser/ui/views/frame/app_menu_button_observer.h"
Expand Down Expand Up @@ -47,7 +48,7 @@ void AppMenuButton::CloseMenu() {
}

void AppMenuButton::OnMenuClosed() {
HandleMenuClosed();
promo_handle_.Release();
for (AppMenuButtonObserver& observer : observer_list_)
observer.AppMenuClosed();
}
Expand All @@ -63,15 +64,31 @@ void AppMenuButton::RunMenu(std::unique_ptr<AppMenuModel> menu_model,
// in the class declaration.
menu_.reset();
menu_model_ = std::move(menu_model);
if (BrowserWindow* browser_window = browser->window()) {
if (auto* controller = browser_window->GetFeaturePromoController()) {
if (auto* promo_specification =
controller->GetCurrentPromoSpecificationForAnchor(
GetProperty(views::kElementIdentifierKey))) {
if (auto highlighted_identifier =
promo_specification->highlighted_menu_identifier()) {
promo_handle_ = browser_window->CloseFeaturePromoAndContinue(
*controller->GetCurrentPromoFeature());

if (promo_handle_.is_valid()) {
menu_model_->SetHighlightedIdentifier(highlighted_identifier);
}
}
}
}
}
menu_model_->Init();

menu_ = std::make_unique<AppMenu>(browser, menu_model_.get(), run_flags);
menu_->RunMenu(menu_button_controller_);

for (AppMenuButtonObserver& observer : observer_list_)
observer.AppMenuShown();
}

void AppMenuButton::HandleMenuClosed() {}

BEGIN_METADATA(AppMenuButton, ToolbarButton)
END_METADATA
6 changes: 2 additions & 4 deletions chrome/browser/ui/views/frame/app_menu_button.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/memory/raw_ptr.h"
#include "base/observer_list.h"
#include "chrome/browser/ui/views/toolbar/toolbar_button.h"
#include "components/user_education/common/feature_promo_handle.h"
#include "ui/base/metadata/metadata_header_macros.h"

class AppMenu;
Expand Down Expand Up @@ -60,10 +61,6 @@ class AppMenuButton : public ToolbarButton {
Browser* browser,
int run_flags);

// Provided for subclasses to handle menu close, before observers are
// notified. Default implementation does nothing.
virtual void HandleMenuClosed();

private:
// App model and menu.
// Note that the menu should be destroyed before the model it uses, so the
Expand All @@ -76,6 +73,7 @@ class AppMenuButton : public ToolbarButton {
base::ObserverList<AppMenuButtonObserver>::Unchecked observer_list_;

raw_ptr<views::MenuButtonController> menu_button_controller_;
user_education::FeaturePromoHandle promo_handle_;
};

#endif // CHROME_BROWSER_UI_VIEWS_FRAME_APP_MENU_BUTTON_H_
22 changes: 4 additions & 18 deletions chrome/browser/ui/views/toolbar/browser_app_menu_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,18 +103,17 @@ void BrowserAppMenuButton::ShowMenu(int run_types) {

Browser* browser = toolbar_view_->browser();

// If the menu was opened while reopen tab in-product help was
// showing, we continue the IPH into the menu. Notify the promo
// controller we are taking control of the promo.
AlertMenuItem alert_item = CloseFeaturePromoAndContinue();
// Allow highlighting menu items when the menu was opened while
// certain tutorials are running.
AlertMenuItem alert_item = GetAlertItemForRunningTutorial();

RunMenu(std::make_unique<AppMenuModel>(
toolbar_view_, browser, toolbar_view_->app_menu_icon_controller(),
alert_item),
browser, run_types);
}

AlertMenuItem BrowserAppMenuButton::CloseFeaturePromoAndContinue() {
AlertMenuItem BrowserAppMenuButton::GetAlertItemForRunningTutorial() {
Browser* browser = toolbar_view_->browser();
BrowserWindow* browser_window = browser->window();

Expand All @@ -128,12 +127,6 @@ AlertMenuItem BrowserAppMenuButton::CloseFeaturePromoAndContinue() {
return AlertMenuItem::kPasswordManager;
}

promo_handle_ = browser_window->CloseFeaturePromoAndContinue(
feature_engagement::kIPHHighEfficiencyModeFeature);

if (promo_handle_.is_valid())
return AlertMenuItem::kPerformance;

return AlertMenuItem::kNone;
}

Expand Down Expand Up @@ -206,13 +199,6 @@ SkColor BrowserAppMenuButton::GetForegroundColor(ButtonState state) const {
return ToolbarButton::GetForegroundColor(state);
}

void BrowserAppMenuButton::HandleMenuClosed() {
// If we were showing a promo in the menu, drop the handle to notify
// FeaturePromoController we're done. This is a no-op if we weren't
// showing the promo.
promo_handle_.Release();
}

void BrowserAppMenuButton::UpdateTextAndHighlightColor() {
int tooltip_message_id;
std::u16string text;
Expand Down
8 changes: 4 additions & 4 deletions chrome/browser/ui/views/toolbar/browser_app_menu_button.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ class BrowserAppMenuButton : public AppMenuButton {
void OnThemeChanged() override;
// Updates the presentation according to |severity_| and the theme provider.
void UpdateIcon() override;
void HandleMenuClosed() override;

private:
void OnTouchUiChanged();
Expand All @@ -71,9 +70,10 @@ class BrowserAppMenuButton : public AppMenuButton {
// Sets the padding values depending on whether label is visible.
void UpdateLayoutInsets();

// Closes and continue the flow of an in-product help promo; Returns
// AlertMenuItem which indicates the app menu item that should be alerted.
AlertMenuItem CloseFeaturePromoAndContinue();
// TODO(mickeyburks): Highlight menu items through TutorialDescription
// Returns an AlertMenuItem which indicates the app menu item that
// should be alerted while certain tutorials are running.
AlertMenuItem GetAlertItemForRunningTutorial();

AppMenuIconController::TypeAndSeverity type_and_severity_{
AppMenuIconController::IconType::NONE,
Expand Down

0 comments on commit b0aa105

Please sign in to comment.