Commit 7f00d9bf authored by Dana Fried's avatar Dana Fried Committed by Commit Bot

Plumb 'New' command status through simple menu model.

This was already added to menu item in a previous CL (behind a flag).

This CL allows SimpleMenuModel to specify that a menu item should be
flagged as "New" to help educate users about new features in Chrome.

Bug: 1098731
Change-Id: Ib2fd08acebb8731bcbfad83678017cf55f540b76
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2264196
Commit-Queue: Dana Fried <dfried@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarMichael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782170}
parent 3394c050
...@@ -19,6 +19,10 @@ bool MenuModel::IsVisibleAt(int index) const { ...@@ -19,6 +19,10 @@ bool MenuModel::IsVisibleAt(int index) const {
return true; return true;
} }
bool MenuModel::IsNewFeatureAt(int index) const {
return false;
}
// static // static
bool MenuModel::GetModelAndIndexForCommandId(int command_id, bool MenuModel::GetModelAndIndexForCommandId(int command_id,
MenuModel** model, MenuModel** model,
......
...@@ -113,6 +113,11 @@ class COMPONENT_EXPORT(UI_BASE) MenuModel ...@@ -113,6 +113,11 @@ class COMPONENT_EXPORT(UI_BASE) MenuModel
// Returns true if the menu item is visible. // Returns true if the menu item is visible.
virtual bool IsVisibleAt(int index) const; virtual bool IsVisibleAt(int index) const;
// Returns true if the menu item grants access to a new feature that we want
// to show off to users (items marked as new will receive a "New" badge when
// the appropriate flag is enabled).
virtual bool IsNewFeatureAt(int index) const;
// Returns the model for the submenu at the specified index. // Returns the model for the submenu at the specified index.
virtual MenuModel* GetSubmenuModelAt(int index) const = 0; virtual MenuModel* GetSubmenuModelAt(int index) const = 0;
......
...@@ -317,6 +317,10 @@ void SimpleMenuModel::SetVisibleAt(int index, bool visible) { ...@@ -317,6 +317,10 @@ void SimpleMenuModel::SetVisibleAt(int index, bool visible) {
MenuItemsChanged(); MenuItemsChanged();
} }
void SimpleMenuModel::SetIsNewFeatureAt(int index, bool is_new_feature) {
items_[ValidateItemIndex(index)].is_new_feature = is_new_feature;
}
void SimpleMenuModel::Clear() { void SimpleMenuModel::Clear() {
items_.clear(); items_.clear();
MenuItemsChanged(); MenuItemsChanged();
...@@ -432,6 +436,10 @@ bool SimpleMenuModel::IsVisibleAt(int index) const { ...@@ -432,6 +436,10 @@ bool SimpleMenuModel::IsVisibleAt(int index) const {
items_[ValidateItemIndex(index)].visible; items_[ValidateItemIndex(index)].visible;
} }
bool SimpleMenuModel::IsNewFeatureAt(int index) const {
return items_[ValidateItemIndex(index)].is_new_feature;
}
void SimpleMenuModel::ActivatedAt(int index) { void SimpleMenuModel::ActivatedAt(int index) {
ActivatedAt(index, 0); ActivatedAt(index, 0);
} }
......
...@@ -159,6 +159,9 @@ class COMPONENT_EXPORT(UI_BASE) SimpleMenuModel : public MenuModel { ...@@ -159,6 +159,9 @@ class COMPONENT_EXPORT(UI_BASE) SimpleMenuModel : public MenuModel {
// Sets whether the item at |index| is visible. // Sets whether the item at |index| is visible.
void SetVisibleAt(int index, bool visible); void SetVisibleAt(int index, bool visible);
// Sets whether the item at |index| is new.
void SetIsNewFeatureAt(int index, bool is_new_feature);
// Clears all items. Note that it does not free MenuModel of submenu. // Clears all items. Note that it does not free MenuModel of submenu.
void Clear(); void Clear();
...@@ -183,6 +186,7 @@ class COMPONENT_EXPORT(UI_BASE) SimpleMenuModel : public MenuModel { ...@@ -183,6 +186,7 @@ class COMPONENT_EXPORT(UI_BASE) SimpleMenuModel : public MenuModel {
ui::ButtonMenuItemModel* GetButtonMenuItemAt(int index) const override; ui::ButtonMenuItemModel* GetButtonMenuItemAt(int index) const override;
bool IsEnabledAt(int index) const override; bool IsEnabledAt(int index) const override;
bool IsVisibleAt(int index) const override; bool IsVisibleAt(int index) const override;
bool IsNewFeatureAt(int index) const override;
void ActivatedAt(int index) override; void ActivatedAt(int index) override;
void ActivatedAt(int index, int event_flags) override; void ActivatedAt(int index, int event_flags) override;
MenuModel* GetSubmenuModelAt(int index) const override; MenuModel* GetSubmenuModelAt(int index) const override;
...@@ -216,6 +220,7 @@ class COMPONENT_EXPORT(UI_BASE) SimpleMenuModel : public MenuModel { ...@@ -216,6 +220,7 @@ class COMPONENT_EXPORT(UI_BASE) SimpleMenuModel : public MenuModel {
MenuSeparatorType separator_type = NORMAL_SEPARATOR; MenuSeparatorType separator_type = NORMAL_SEPARATOR;
bool enabled = true; bool enabled = true;
bool visible = true; bool visible = true;
bool is_new_feature = false;
}; };
typedef std::vector<Item> ItemVector; typedef std::vector<Item> ItemVector;
......
...@@ -147,6 +147,20 @@ TEST(SimpleMenuModelTest, IsVisibleAtWithDelegateAndCommandNotVisible) { ...@@ -147,6 +147,20 @@ TEST(SimpleMenuModelTest, IsVisibleAtWithDelegateAndCommandNotVisible) {
ASSERT_FALSE(simple_menu_model.IsEnabledAt(0)); ASSERT_FALSE(simple_menu_model.IsEnabledAt(0));
} }
TEST(SimpleMenuModelTest, SetIsNewFeatureAt) {
SimpleMenuModel simple_menu_model(nullptr);
simple_menu_model.AddItem(/*command_id*/ 5,
base::ASCIIToUTF16("menu item 0"));
simple_menu_model.AddItem(/*command_id*/ 6,
base::ASCIIToUTF16("menu item 1"));
simple_menu_model.SetIsNewFeatureAt(/*index*/ 0, false);
simple_menu_model.SetIsNewFeatureAt(/*index*/ 1, true);
ASSERT_FALSE(simple_menu_model.IsNewFeatureAt(0));
ASSERT_TRUE(simple_menu_model.IsNewFeatureAt(1));
}
TEST(SimpleMenuModelTest, HasIconsViaDelegate) { TEST(SimpleMenuModelTest, HasIconsViaDelegate) {
DelegateBase delegate; DelegateBase delegate;
SimpleMenuModel simple_menu_model(&delegate); SimpleMenuModel simple_menu_model(&delegate);
......
...@@ -107,7 +107,7 @@ MenuItemView* MenuModelAdapter::AddMenuItemFromModelAt(ui::MenuModel* model, ...@@ -107,7 +107,7 @@ MenuItemView* MenuModelAdapter::AddMenuItemFromModelAt(ui::MenuModel* model,
ui::ImageModel icon = model->GetIconAt(model_index); ui::ImageModel icon = model->GetIconAt(model_index);
ui::ImageModel minor_icon = model->GetMinorIconAt(model_index); ui::ImageModel minor_icon = model->GetMinorIconAt(model_index);
return menu->AddMenuItemAt( auto* menu_item_view = menu->AddMenuItemAt(
menu_index, item_id, model->GetLabelAt(model_index), menu_index, item_id, model->GetLabelAt(model_index),
model->GetSecondaryLabelAt(model_index), model->GetSecondaryLabelAt(model_index),
model->GetMinorTextAt(model_index), model->GetMinorTextAt(model_index),
...@@ -118,6 +118,8 @@ MenuItemView* MenuModelAdapter::AddMenuItemFromModelAt(ui::MenuModel* model, ...@@ -118,6 +118,8 @@ MenuItemView* MenuModelAdapter::AddMenuItemFromModelAt(ui::MenuModel* model,
icon.IsVectorIcon() ? ui::ThemedVectorIcon(icon.GetVectorIcon()) icon.IsVectorIcon() ? ui::ThemedVectorIcon(icon.GetVectorIcon())
: ui::ThemedVectorIcon(), : ui::ThemedVectorIcon(),
*type, ui::NORMAL_SEPARATOR); *type, ui::NORMAL_SEPARATOR);
menu_item_view->set_is_new(model->IsNewFeatureAt(model_index));
return menu_item_view;
} }
// Static. // Static.
......
...@@ -78,6 +78,10 @@ class MenuModelBase : public ui::MenuModel { ...@@ -78,6 +78,10 @@ class MenuModelBase : public ui::MenuModel {
bool IsVisibleAt(int index) const override { return items_[index].visible; } bool IsVisibleAt(int index) const override { return items_[index].visible; }
bool IsNewFeatureAt(int index) const override {
return items_[index].new_feature;
}
MenuModel* GetSubmenuModelAt(int index) const override { MenuModel* GetSubmenuModelAt(int index) const override {
return items_[index].submenu; return items_[index].submenu;
} }
...@@ -117,6 +121,7 @@ class MenuModelBase : public ui::MenuModel { ...@@ -117,6 +121,7 @@ class MenuModelBase : public ui::MenuModel {
ui::MenuModel* submenu; ui::MenuModel* submenu;
bool enabled; bool enabled;
bool visible; bool visible;
bool new_feature = false;
}; };
const Item& GetItemDefinition(size_t index) { return items_[index]; } const Item& GetItemDefinition(size_t index) { return items_[index]; }
...@@ -155,6 +160,7 @@ class ActionableSubmenuModel : public MenuModelBase { ...@@ -155,6 +160,7 @@ class ActionableSubmenuModel : public MenuModelBase {
ActionableSubmenuModel() : MenuModelBase(kActionableSubmenuIdBase) { ActionableSubmenuModel() : MenuModelBase(kActionableSubmenuIdBase) {
items_.emplace_back(TYPE_COMMAND, "actionable submenu item 0", nullptr); items_.emplace_back(TYPE_COMMAND, "actionable submenu item 0", nullptr);
items_.emplace_back(TYPE_COMMAND, "actionable submenu item 1", nullptr); items_.emplace_back(TYPE_COMMAND, "actionable submenu item 1", nullptr);
items_[1].new_feature = true;
} }
~ActionableSubmenuModel() override = default; ~ActionableSubmenuModel() override = default;
...@@ -246,6 +252,9 @@ void CheckSubmenu(const RootModel& model, ...@@ -246,6 +252,9 @@ void CheckSubmenu(const RootModel& model,
// Check visibility. // Check visibility.
EXPECT_EQ(model_item.visible, item->GetVisible()); EXPECT_EQ(model_item.visible, item->GetVisible());
// Check new feature flag.
EXPECT_EQ(model_item.new_feature, item->is_new());
// Check activation. // Check activation.
static_cast<views::MenuDelegate*>(delegate)->ExecuteCommand(id); static_cast<views::MenuDelegate*>(delegate)->ExecuteCommand(id);
EXPECT_EQ(i, size_t{submodel->last_activation()}); EXPECT_EQ(i, size_t{submodel->last_activation()});
...@@ -324,6 +333,9 @@ TEST_F(MenuModelAdapterTest, BasicTest) { ...@@ -324,6 +333,9 @@ TEST_F(MenuModelAdapterTest, BasicTest) {
// Check visibility. // Check visibility.
EXPECT_EQ(model_item.visible, item->GetVisible()); EXPECT_EQ(model_item.visible, item->GetVisible());
// Check visibility.
EXPECT_EQ(model_item.new_feature, item->is_new());
// Check activation. // Check activation.
static_cast<views::MenuDelegate*>(&delegate)->ExecuteCommand(id); static_cast<views::MenuDelegate*>(&delegate)->ExecuteCommand(id);
EXPECT_EQ(i, size_t{model.last_activation()}); EXPECT_EQ(i, size_t{model.last_activation()});
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment