Commit 669aa2e1 authored by Angela Yoeurng's avatar Angela Yoeurng Committed by Commit Bot

[Omnibox] Move Remove suggestion button to end of row

This CL moves the Remove Suggestion Button to be after the
suggestion button row when navigating via Tab

Bug: 1111205
Change-Id: I7abe2ebf8e03e83b4a593551134d05caba4cdf6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2328296Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Angela Yoeurng <yoangela@chromium.org>
Cr-Commit-Position: refs/heads/master@{#793385}
parent a629ac76
...@@ -349,10 +349,10 @@ OmniboxPopupModel::GetAllAvailableSelectionsSorted(Direction direction, ...@@ -349,10 +349,10 @@ OmniboxPopupModel::GetAllAvailableSelectionsSorted(Direction direction,
if (OmniboxFieldTrial::IsSuggestionButtonRowEnabled()) { if (OmniboxFieldTrial::IsSuggestionButtonRowEnabled()) {
// The button row experiment makes things simple. We no longer access // The button row experiment makes things simple. We no longer access
// keyword mode tab button in this case. // keyword mode tab button in this case.
all_states.push_back(FOCUSED_BUTTON_REMOVE_SUGGESTION);
all_states.push_back(FOCUSED_BUTTON_KEYWORD); all_states.push_back(FOCUSED_BUTTON_KEYWORD);
all_states.push_back(FOCUSED_BUTTON_TAB_SWITCH); all_states.push_back(FOCUSED_BUTTON_TAB_SWITCH);
all_states.push_back(FOCUSED_BUTTON_PEDAL); all_states.push_back(FOCUSED_BUTTON_PEDAL);
all_states.push_back(FOCUSED_BUTTON_REMOVE_SUGGESTION);
} else { } else {
// Keyword mode is only accessible by Tabbing forward. // Keyword mode is only accessible by Tabbing forward.
if (direction == kForward) { if (direction == kForward) {
...@@ -360,8 +360,8 @@ OmniboxPopupModel::GetAllAvailableSelectionsSorted(Direction direction, ...@@ -360,8 +360,8 @@ OmniboxPopupModel::GetAllAvailableSelectionsSorted(Direction direction,
all_states.push_back(KEYWORD); all_states.push_back(KEYWORD);
} }
} }
all_states.push_back(FOCUSED_BUTTON_REMOVE_SUGGESTION);
all_states.push_back(FOCUSED_BUTTON_TAB_SWITCH); all_states.push_back(FOCUSED_BUTTON_TAB_SWITCH);
all_states.push_back(FOCUSED_BUTTON_REMOVE_SUGGESTION);
} }
} }
DCHECK(std::is_sorted(all_states.begin(), all_states.end())) DCHECK(std::is_sorted(all_states.begin(), all_states.end()))
...@@ -498,15 +498,6 @@ bool OmniboxPopupModel::IsControlPresentOnMatch(Selection selection) const { ...@@ -498,15 +498,6 @@ bool OmniboxPopupModel::IsControlPresentOnMatch(Selection selection) const {
return true; return true;
case KEYWORD: case KEYWORD:
return match.associated_keyword != nullptr; return match.associated_keyword != nullptr;
case FOCUSED_BUTTON_REMOVE_SUGGESTION:
// Remove suggestion buttons are suppressed for matches with an associated
// keyword or tab match, unless dedicated button row is enabled.
if (OmniboxFieldTrial::IsSuggestionButtonRowEnabled())
return match.SupportsDeletion();
else
return !match.associated_keyword &&
!match.ShouldShowTabMatchButtonInlineInResultView() &&
match.SupportsDeletion();
case FOCUSED_BUTTON_KEYWORD: case FOCUSED_BUTTON_KEYWORD:
return match.associated_keyword != nullptr; return match.associated_keyword != nullptr;
case FOCUSED_BUTTON_TAB_SWITCH: case FOCUSED_BUTTON_TAB_SWITCH:
...@@ -518,6 +509,15 @@ bool OmniboxPopupModel::IsControlPresentOnMatch(Selection selection) const { ...@@ -518,6 +509,15 @@ bool OmniboxPopupModel::IsControlPresentOnMatch(Selection selection) const {
return match.ShouldShowTabMatchButtonInlineInResultView(); return match.ShouldShowTabMatchButtonInlineInResultView();
case FOCUSED_BUTTON_PEDAL: case FOCUSED_BUTTON_PEDAL:
return match.pedal != nullptr; return match.pedal != nullptr;
case FOCUSED_BUTTON_REMOVE_SUGGESTION:
// Remove suggestion buttons are suppressed for matches with an associated
// keyword or tab match, unless dedicated button row is enabled.
if (OmniboxFieldTrial::IsSuggestionButtonRowEnabled())
return match.SupportsDeletion();
else
return !match.associated_keyword &&
!match.ShouldShowTabMatchButtonInlineInResultView() &&
match.SupportsDeletion();
default: default:
break; break;
} }
...@@ -612,9 +612,6 @@ base::string16 OmniboxPopupModel::GetAccessibilityLabelForCurrentSelection( ...@@ -612,9 +612,6 @@ base::string16 OmniboxPopupModel::GetAccessibilityLabelForCurrentSelection(
// TODO(tommycli): Investigate whether the accessibility messaging for // TODO(tommycli): Investigate whether the accessibility messaging for
// Keyword mode belongs here. // Keyword mode belongs here.
break; break;
case FOCUSED_BUTTON_REMOVE_SUGGESTION:
additional_message_id = IDS_ACC_REMOVE_SUGGESTION_FOCUSED_PREFIX;
break;
case FOCUSED_BUTTON_KEYWORD: case FOCUSED_BUTTON_KEYWORD:
// TODO(yoangela): Add an accessibility message for the Keyword button // TODO(yoangela): Add an accessibility message for the Keyword button
// in the button-row UI configuration. // in the button-row UI configuration.
...@@ -626,6 +623,9 @@ base::string16 OmniboxPopupModel::GetAccessibilityLabelForCurrentSelection( ...@@ -626,6 +623,9 @@ base::string16 OmniboxPopupModel::GetAccessibilityLabelForCurrentSelection(
// TODO(orinj): Add an accessibility message for the Pedal button // TODO(orinj): Add an accessibility message for the Pedal button
// in the button-row UI configuration. // in the button-row UI configuration.
break; break;
case FOCUSED_BUTTON_REMOVE_SUGGESTION:
additional_message_id = IDS_ACC_REMOVE_SUGGESTION_FOCUSED_PREFIX;
break;
default: default:
break; break;
} }
......
...@@ -65,23 +65,22 @@ class OmniboxPopupModel { ...@@ -65,23 +65,22 @@ class OmniboxPopupModel {
// FOCUSED_BUTTON_KEYWORD state, which is only for button focus. // FOCUSED_BUTTON_KEYWORD state, which is only for button focus.
KEYWORD = 2, KEYWORD = 2,
// FOCUSED_BUTTON_REMOVE_SUGGESTION state means the Remove Suggestion (X)
// button is focused. Pressing enter will attempt to remove this suggestion.
FOCUSED_BUTTON_REMOVE_SUGGESTION = 3,
// FOCUSED_BUTTON_KEYWORD is used when the keyword button is in focus, not // FOCUSED_BUTTON_KEYWORD is used when the keyword button is in focus, not
// actually in Keyword Mode. This is currently only used if deciated button // actually in Keyword Mode. This is currently only used if deciated button
// row is enabled // row is enabled
FOCUSED_BUTTON_KEYWORD = 4, FOCUSED_BUTTON_KEYWORD = 3,
// FOCUSED_BUTTON_TAB_SWITCH state means the Switch Tab button is focused. // FOCUSED_BUTTON_TAB_SWITCH state means the Switch Tab button is focused.
// Pressing enter will switch to the tab match. // Pressing enter will switch to the tab match.
FOCUSED_BUTTON_TAB_SWITCH = 5, FOCUSED_BUTTON_TAB_SWITCH = 4,
// FOCUSED_BUTTON_PEDAL state means a Pedal button is in focus. This is // FOCUSED_BUTTON_PEDAL state means a Pedal button is in focus. This is
// currently only used when dedicated button row and pedals are enabled. // currently only used when dedicated button row and pedals are enabled.
FOCUSED_BUTTON_PEDAL = 6, FOCUSED_BUTTON_PEDAL = 5,
// FOCUSED_BUTTON_REMOVE_SUGGESTION state means the Remove Suggestion (X)
// button is focused. Pressing enter will attempt to remove this suggestion.
FOCUSED_BUTTON_REMOVE_SUGGESTION = 6,
}; };
struct Selection { struct Selection {
......
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