Commit 9622f41d authored by Orin Jaworski's avatar Orin Jaworski Committed by Commit Bot

[omnibox] Introduce three distinct button focus line states

This CL adds three new focus states to the popup model line state enum
for use when suggestion button row is enabled. The forward and back
step logic is updated with care to respect the experimental flag, and
to prepare for new keyword logic handling.

Bug: 1046523
Change-Id: I336f74f3d1ed971ee23c245df196192ef29d0940
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135381Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Commit-Queue: Orin Jaworski <orinj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756905}
parent 479399dd
...@@ -128,17 +128,24 @@ void OmniboxPopupModel::ComputeMatchMaxWidths(int contents_width, ...@@ -128,17 +128,24 @@ void OmniboxPopupModel::ComputeMatchMaxWidths(int contents_width,
OmniboxPopupModel::LineState OmniboxPopupModel::GetNextLineState( OmniboxPopupModel::LineState OmniboxPopupModel::GetNextLineState(
LineState state, LineState state,
Direction direction) { Direction direction) {
const bool button_row = OmniboxFieldTrial::IsSuggestionButtonRowEnabled();
switch (direction) { switch (direction) {
case kForward: case kForward:
switch (state) { switch (state) {
case NO_STATE: case NO_STATE:
return NORMAL; return NORMAL;
case NORMAL: case NORMAL:
return KEYWORD; return button_row ? FOCUSED_BUTTON_KEYWORD : KEYWORD;
case KEYWORD: case KEYWORD:
return BUTTON_FOCUSED; return button_row ? FOCUSED_BUTTON_TAB_SWITCH : BUTTON_FOCUSED;
case BUTTON_FOCUSED: case BUTTON_FOCUSED:
return NO_STATE; return NO_STATE;
case FOCUSED_BUTTON_KEYWORD:
return FOCUSED_BUTTON_TAB_SWITCH;
case FOCUSED_BUTTON_TAB_SWITCH:
return FOCUSED_BUTTON_PEDAL;
case FOCUSED_BUTTON_PEDAL:
return NO_STATE;
default: default:
break; break;
} }
...@@ -146,13 +153,19 @@ OmniboxPopupModel::LineState OmniboxPopupModel::GetNextLineState( ...@@ -146,13 +153,19 @@ OmniboxPopupModel::LineState OmniboxPopupModel::GetNextLineState(
case kBackward: case kBackward:
switch (state) { switch (state) {
case NO_STATE: case NO_STATE:
return BUTTON_FOCUSED; return button_row ? FOCUSED_BUTTON_PEDAL : BUTTON_FOCUSED;
case NORMAL: case NORMAL:
return NO_STATE; return NO_STATE;
case KEYWORD: case KEYWORD:
return NORMAL; return NORMAL;
case BUTTON_FOCUSED: case BUTTON_FOCUSED:
return KEYWORD; return KEYWORD;
case FOCUSED_BUTTON_KEYWORD:
return NORMAL;
case FOCUSED_BUTTON_TAB_SWITCH:
return FOCUSED_BUTTON_KEYWORD;
case FOCUSED_BUTTON_PEDAL:
return FOCUSED_BUTTON_TAB_SWITCH;
default: default:
break; break;
} }
...@@ -447,17 +460,23 @@ bool OmniboxPopupModel::IsSelectionAvailable(Selection selection) const { ...@@ -447,17 +460,23 @@ bool OmniboxPopupModel::IsSelectionAvailable(Selection selection) const {
} }
const auto& match = result().match_at(selection.line); const auto& match = result().match_at(selection.line);
switch (selection.state) { switch (selection.state) {
case OmniboxPopupModel::NO_STATE: case NO_STATE:
return false; return false;
case OmniboxPopupModel::NORMAL: case NORMAL:
return true; return true;
case OmniboxPopupModel::KEYWORD: case KEYWORD:
return match.associated_keyword != nullptr; return match.associated_keyword != nullptr;
case OmniboxPopupModel::BUTTON_FOCUSED: case BUTTON_FOCUSED:
// TODO(orinj): Here is an opportunity to clean up the presentational // TODO(orinj): Here is an opportunity to clean up the presentational
// logic that pkasting wanted to take out of AutocompleteMatch. The view // logic that pkasting wanted to take out of AutocompleteMatch. The view
// should be driven by the model, so this is really the place to decide. // should be driven by the model, so this is really the place to decide.
return match.ShouldShowTabMatchButton(); return match.ShouldShowTabMatchButton();
case FOCUSED_BUTTON_KEYWORD:
return match.associated_keyword != nullptr;
case FOCUSED_BUTTON_TAB_SWITCH:
return match.has_tab_match;
case FOCUSED_BUTTON_PEDAL:
return match.pedal != nullptr;
default: default:
break; break;
} }
......
...@@ -55,9 +55,20 @@ class OmniboxPopupModel { ...@@ -55,9 +55,20 @@ class OmniboxPopupModel {
// See |Selection::state| below for details. // See |Selection::state| below for details.
enum LineState { enum LineState {
NORMAL, NORMAL,
// KEYWORD state means actually in keyword mode, as distinct from the
// FOCUSED_BUTTON_KEYWORD state, which is only for button focus.
KEYWORD, KEYWORD,
// The single (ambiguous) button focus state is not used when button row
// is enabled. Instead, the specific FOCUSED_* states below apply.
BUTTON_FOCUSED, BUTTON_FOCUSED,
// Button row focus states:
FOCUSED_BUTTON_KEYWORD,
FOCUSED_BUTTON_TAB_SWITCH,
FOCUSED_BUTTON_PEDAL,
// NO_STATE logically indicates unavailability of a state, and is // NO_STATE logically indicates unavailability of a state, and is
// only used internally. NO_STATE values are not persisted in members, // only used internally. NO_STATE values are not persisted in members,
// are not returned from public methods, and should not be used by // are not returned from public methods, and should not be used by
......
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