Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(622)

Unified Diff: ui/views/controls/menu/menu_controller_unittest.cc

Issue 1230163004: Selects last item in a menu when pressing 'up' initially (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Selects last item in a menu when pressing 'up' initially (comments) Created 5 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: ui/views/controls/menu/menu_controller_unittest.cc
diff --git a/ui/views/controls/menu/menu_controller_unittest.cc b/ui/views/controls/menu/menu_controller_unittest.cc
index 1fd47a55a8eaf19a4be82451077a4092641fd9d2..604fcf18a4b1aa19998f3aa3aa2c924d12c101b6 100644
--- a/ui/views/controls/menu/menu_controller_unittest.cc
+++ b/ui/views/controls/menu/menu_controller_unittest.cc
@@ -46,6 +46,26 @@ class TestMenuItemView : public MenuItemView {
DISALLOW_COPY_AND_ASSIGN(TestMenuItemView);
};
+class SubmenuViewShown : public SubmenuView {
+ public:
+ SubmenuViewShown(MenuItemView* parent) : SubmenuView(parent) {}
+ ~SubmenuViewShown() override {}
+ bool IsShowing() override { return true; }
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(SubmenuViewShown);
+};
+
+class TestMenuItemViewShown : public MenuItemView {
+ public:
+ TestMenuItemViewShown() : MenuItemView(nullptr) {}
sadrul 2015/07/15 19:21:52 Can it set |submenu_| here? Would that allow not m
varkha 2015/07/15 19:51:34 It is probably possible but the way CreateSubmenu
varkha 2015/07/16 16:32:33 I did that in Patch Set #4. Please see if you like
+ ~TestMenuItemViewShown() override {}
+ SubmenuView* CreateSubmenu() override { return new SubmenuViewShown(this); }
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(TestMenuItemViewShown);
+};
+
class TestPlatformEventSource : public ui::PlatformEventSource {
public:
TestPlatformEventSource() {
@@ -170,6 +190,7 @@ class MenuControllerTest : public ViewsTestBase {
void SetPendingStateItem(MenuItemView* item) {
controller_->pending_state_.item = item;
+ controller_->pending_state_.submenu_open = true;
}
void ResetSelection() {
@@ -182,8 +203,8 @@ class MenuControllerTest : public ViewsTestBase {
controller_->IncrementSelection(delta);
}
- MenuItemView* FindFirstSelectableMenuItem(MenuItemView* parent) {
- return controller_->FindFirstSelectableMenuItem(parent);
+ MenuItemView* FindInitialSelectableMenuItem(MenuItemView* parent, int delta) {
+ return controller_->FindInitialSelectableMenuItem(parent, delta);
}
MenuItemView* FindNextSelectableMenuItem(MenuItemView* parent,
@@ -375,23 +396,80 @@ TEST_F(MenuControllerTest, TouchIdsReleasedCorrectly) {
}
#endif // defined(USE_X11)
-TEST_F(MenuControllerTest, FirstSelectedItem) {
+TEST_F(MenuControllerTest, InitialSelectedItem) {
scoped_ptr<Widget> owner(CreateOwnerWidget());
scoped_ptr<TestMenuItemView> menu_item(new TestMenuItemView);
menu_item->AppendMenuItemWithLabel(1, base::ASCIIToUTF16("One"));
menu_item->AppendMenuItemWithLabel(2, base::ASCIIToUTF16("Two"));
- // Disabling the item "One" gets it skipped when a menu is first opened.
- menu_item->GetSubmenu()->GetMenuItemAt(0)->SetEnabled(false);
-
+ menu_item->AppendMenuItemWithLabel(3, base::ASCIIToUTF16("Three"));
SetupMenu(owner.get(), menu_item.get());
- // First selectable item should be item "Two".
- MenuItemView* first_selectable = FindFirstSelectableMenuItem(menu_item.get());
+ // Leave items "Two" and "Three" enabled.
+ menu_item->GetSubmenu()->GetMenuItemAt(0)->SetEnabled(false);
+ // The first selectable item should be item "Two".
+ MenuItemView* first_selectable =
+ FindInitialSelectableMenuItem(menu_item.get(), 1);
+ ASSERT_NE(nullptr, first_selectable);
+ EXPECT_EQ(2, first_selectable->GetCommand());
+ // The last selectable item should be item "Three".
+ MenuItemView* last_selectable =
+ FindInitialSelectableMenuItem(menu_item.get(), -1);
+ ASSERT_NE(nullptr, last_selectable);
+ EXPECT_EQ(3, last_selectable->GetCommand());
+
+ // Leave items "One" and "Two" enabled.
+ menu_item->GetSubmenu()->GetMenuItemAt(0)->SetEnabled(true);
+ menu_item->GetSubmenu()->GetMenuItemAt(1)->SetEnabled(true);
+ menu_item->GetSubmenu()->GetMenuItemAt(2)->SetEnabled(false);
+ // The first selectable item should be item "One".
+ first_selectable = FindInitialSelectableMenuItem(menu_item.get(), 1);
+ ASSERT_NE(nullptr, first_selectable);
+ EXPECT_EQ(1, first_selectable->GetCommand());
+ // The last selectable item should be item "Two".
+ last_selectable = FindInitialSelectableMenuItem(menu_item.get(), -1);
+ ASSERT_NE(nullptr, last_selectable);
+ EXPECT_EQ(2, last_selectable->GetCommand());
+
+ // Leave only a single item "One" enabled.
+ menu_item->GetSubmenu()->GetMenuItemAt(0)->SetEnabled(true);
+ menu_item->GetSubmenu()->GetMenuItemAt(1)->SetEnabled(false);
+ menu_item->GetSubmenu()->GetMenuItemAt(2)->SetEnabled(false);
+ // The first selectable item should be item "One".
+ first_selectable = FindInitialSelectableMenuItem(menu_item.get(), 1);
+ ASSERT_NE(nullptr, first_selectable);
+ EXPECT_EQ(1, first_selectable->GetCommand());
+ // The last selectable item should be item "One".
+ last_selectable = FindInitialSelectableMenuItem(menu_item.get(), -1);
+ ASSERT_NE(nullptr, last_selectable);
+ EXPECT_EQ(1, last_selectable->GetCommand());
+
+ // Leave only a single item "Three" enabled.
+ menu_item->GetSubmenu()->GetMenuItemAt(0)->SetEnabled(false);
+ menu_item->GetSubmenu()->GetMenuItemAt(1)->SetEnabled(false);
+ menu_item->GetSubmenu()->GetMenuItemAt(2)->SetEnabled(true);
+ // The first selectable item should be item "Three".
+ first_selectable = FindInitialSelectableMenuItem(menu_item.get(), 1);
+ ASSERT_NE(nullptr, first_selectable);
+ EXPECT_EQ(3, first_selectable->GetCommand());
+ // The last selectable item should be item "Three".
+ last_selectable = FindInitialSelectableMenuItem(menu_item.get(), -1);
+ ASSERT_NE(nullptr, last_selectable);
+ EXPECT_EQ(3, last_selectable->GetCommand());
+
+ // Leave only a single item ("Two") selected. It should be the first and the
+ // last selectable item.
+ menu_item->GetSubmenu()->GetMenuItemAt(0)->SetEnabled(false);
+ menu_item->GetSubmenu()->GetMenuItemAt(1)->SetEnabled(true);
+ menu_item->GetSubmenu()->GetMenuItemAt(2)->SetEnabled(false);
+ first_selectable = FindInitialSelectableMenuItem(menu_item.get(), 1);
+ ASSERT_NE(nullptr, first_selectable);
EXPECT_EQ(2, first_selectable->GetCommand());
+ last_selectable = FindInitialSelectableMenuItem(menu_item.get(), -1);
+ ASSERT_NE(nullptr, last_selectable);
+ EXPECT_EQ(2, last_selectable->GetCommand());
// There should be no next or previous selectable item since there is only a
// single enabled item in the menu.
- SetPendingStateItem(first_selectable);
EXPECT_EQ(nullptr, FindNextSelectableMenuItem(menu_item.get(), 1, 1));
EXPECT_EQ(nullptr, FindNextSelectableMenuItem(menu_item.get(), 1, -1));
@@ -445,4 +523,25 @@ TEST_F(MenuControllerTest, NextSelectedItem) {
ResetSelection();
}
+TEST_F(MenuControllerTest, NextSelectedItemUp) {
+ scoped_ptr<Widget> owner(CreateOwnerWidget());
+ scoped_ptr<TestMenuItemViewShown> menu_item(new TestMenuItemViewShown);
+ menu_item->AppendMenuItemWithLabel(1, base::ASCIIToUTF16("One"));
+ menu_item->AppendMenuItemWithLabel(2, base::ASCIIToUTF16("Two"));
+ menu_item->AppendMenuItemWithLabel(3, base::ASCIIToUTF16("Three"));
+ menu_item->AppendMenuItemWithLabel(4, base::ASCIIToUTF16("Four"));
+ // Disabling the item "Four" gets it skipped when using keyboard to navigate.
+ menu_item->GetSubmenu()->GetMenuItemAt(3)->SetEnabled(false);
+
+ SetupMenu(owner.get(), menu_item.get());
+
+ // Fake initial root item selection and submenu showing.
+ SetPendingStateItem(menu_item.get());
+ EXPECT_EQ(0, pending_state_item()->GetCommand());
+
+ // Move up and select a previous (in our case the last enabled) item.
+ IncrementSelection(-1);
+ EXPECT_EQ(3, pending_state_item()->GetCommand());
+}
+
} // namespace views

Powered by Google App Engine
This is Rietveld 408576698