Skip to content

GitLab

T
tangled
Commit 6f06c4ad authored by Olesia Marukhno's avatar Olesia Marukhno Committed by Commit Bot
Browse files
Options

[omnibox] Refactoring OmniboxResultView to use flex layout 

Based on: https://chromium-review.googlesource.com/c/chromium/src/+/1865014

Bug: 1005568
Change-Id: I8c04530c8a0582c2d7446dfcc42ec661d6efae2e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2398500Reviewed-by: default avatarAngela Yoeurng <yoangela@chromium.org>
Reviewed-by: default avatarBret Sepulveda <bsep@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Commit-Queue: Olesia Marukhno <olesiamarukhno@google.com>
Cr-Commit-Position: refs/heads/master@{#821675}
parent 365bd152
Hide whitespace changes
Inline Side-by-side
Showing with 126 additions and 137 deletions
chrome/browser/ui/views/omnibox/omnibox_result_view.cc
View file @ 6f06c4ad
......@@ -42,6 +42,9 @@
#include "ui/views/controls/button/image_button_factory.h"
#include "ui/views/controls/focus_ring.h"
#include "ui/views/controls/highlight_path_generator.h"
#include "ui/views/layout/fill_layout.h"
#include "ui/views/layout/flex_layout.h"
#include "ui/views/view_class_properties.h"
#if defined(OS_WIN)
#include "base/win/atl.h"
......@@ -70,14 +73,16 @@ class OmniboxRemoveSuggestionButton : public views::ImageButton {
} // namespace
////////////////////////////////////////////////////////////////////////////////
// OmniboxResultFocusBar
// OmniboxResultSelectionIndicator
class OmniboxResultFocusBar : public views::View {
class OmniboxResultSelectionIndicator : public views::View {
public:
static constexpr int kStrokeThickness = 3;
explicit OmniboxResultFocusBar(OmniboxResultView* result_view)
: result_view_(result_view) {}
explicit OmniboxResultSelectionIndicator(OmniboxResultView* result_view)
: result_view_(result_view) {
SetPreferredSize({kStrokeThickness, 0});
}
// views::View:
void OnPaint(gfx::Canvas* canvas) override {
......@@ -135,15 +140,42 @@ OmniboxResultView::OmniboxResultView(
base::BindRepeating(&OmniboxResultView::UpdateHoverState,
base::Unretained(this))) {
CHECK_GE(model_index, 0u);
SetLayoutManager(std::make_unique<views::FlexLayout>())
->SetOrientation(views::LayoutOrientation::kVertical);
suggestion_container_ = AddChildView(std::make_unique<views::View>());
suggestion_container_->SetLayoutManager(
std::make_unique<views::FillLayout>());
mouse_enter_exit_handler_.ObserveMouseEnterExitOn(suggestion_container_);
if (OmniboxFieldTrial::IsRefinedFocusStateEnabled()) {
focus_bar_ = AddChildView(std::make_unique<OmniboxResultFocusBar>(this));
// TODO(olesiamarukhno): Consider making it a decoration instead of separate
// view (painting it in a layer).
selection_indicator_ = suggestion_container_->AddChildView(
std::make_unique<OmniboxResultSelectionIndicator>(this));
}
suggestion_view_ = AddChildView(std::make_unique<OmniboxMatchCellView>(this));
suggestion_tab_switch_button_ =
AddChildView(std::make_unique<OmniboxTabSwitchButton>(
views::View* suggestion_button_container =
suggestion_container_->AddChildView(std::make_unique<views::View>());
suggestion_button_container
->SetLayoutManager(std::make_unique<views::FlexLayout>())
->SetCrossAxisAlignment(views::LayoutAlignment::kCenter);
suggestion_button_container->SetProperty(
views::kFlexBehaviorKey,
views::FlexSpecification(views::MinimumFlexSizeRule::kScaleToZero,
views::MaximumFlexSizeRule::kUnbounded));
suggestion_view_ = suggestion_button_container->AddChildView(
std::make_unique<OmniboxMatchCellView>(this));
suggestion_view_->SetProperty(
views::kFlexBehaviorKey,
views::FlexSpecification(views::MinimumFlexSizeRule::kScaleToZero,
views::MaximumFlexSizeRule::kUnbounded)
.WithWeight(4));
const gfx::Insets child_insets(0, 0, 0, OmniboxMatchCellView::kMarginRight);
suggestion_tab_switch_button_ = suggestion_button_container->AddChildView(
std::make_unique<OmniboxTabSwitchButton>(
base::BindRepeating(&OmniboxResultView::ButtonPressed,
base::Unretained(this),
OmniboxPopupModel::FOCUSED_BUTTON_TAB_SWITCH),
......@@ -151,17 +183,22 @@ OmniboxResultView::OmniboxResultView(
l10n_util::GetStringUTF16(IDS_OMNIBOX_TAB_SUGGEST_HINT),
l10n_util::GetStringUTF16(IDS_OMNIBOX_TAB_SUGGEST_SHORT_HINT),
omnibox::kSwitchIcon));
suggestion_tab_switch_button_->SetProperty(views::kMarginsKey, child_insets);
suggestion_tab_switch_button_->SetProperty(
views::kFlexBehaviorKey,
views::FlexSpecification(OmniboxTabSwitchButton::GetFlexRule())
.WithWeight(1));
// This is intentionally not in the tab order by default, but should be if the
// user has full-acessibility mode on. This is because this is a tertiary
// priority button, which already has a Shift+Delete shortcut.
// TODO(tommycli): Make sure we announce the Shift+Delete capability in the
// accessibility node data for removable suggestions.
remove_suggestion_button_ = AddChildView(
remove_suggestion_button_ = suggestion_button_container->AddChildView(
std::make_unique<OmniboxRemoveSuggestionButton>(base::BindRepeating(
&OmniboxResultView::ButtonPressed, base::Unretained(this),
OmniboxPopupModel::FOCUSED_BUTTON_REMOVE_SUGGESTION)));
mouse_enter_exit_handler_.ObserveMouseEnterExitOn(remove_suggestion_button_);
remove_suggestion_button_->SetProperty(views::kMarginsKey, child_insets);
views::InstallCircleHighlightPathGenerator(remove_suggestion_button_);
remove_suggestion_button_->SetTooltipText(
l10n_util::GetStringUTF16(IDS_OMNIBOX_REMOVE_SUGGESTION));
......@@ -176,13 +213,15 @@ OmniboxResultView::OmniboxResultView(
if (OmniboxFieldTrial::IsSuggestionButtonRowEnabled()) {
button_row_ = AddChildView(std::make_unique<OmniboxSuggestionButtonRowView>(
popup_contents_view_, model_index));
// Quickly mouse-exiting through the suggestion button row sometimes leaves
// the whole row highlighted. This fixes that. It doesn't seem necessary to
// further observe the child controls of |button_row_|.
mouse_enter_exit_handler_.ObserveMouseEnterExitOn(button_row_);
}
keyword_view_ = AddChildView(std::make_unique<OmniboxMatchCellView>(this));
keyword_view_ = suggestion_button_container->AddChildView(
std::make_unique<OmniboxMatchCellView>(this));
keyword_view_->SetVisible(false);
keyword_view_->icon()->SetFlipCanvasOnPaintForRTLUI(true);
keyword_view_->icon()->SizeToPreferredSize();
......@@ -215,6 +254,11 @@ void OmniboxResultView::SetMatch(const AutocompleteMatch& match) {
match_ = match.GetMatchWithContentsAndDescriptionPossiblySwapped();
keyword_slide_animation_->Reset();
const int suggestion_indent =
popup_contents_view_->InExplicitExperimentalKeywordMode() ? 70 : 0;
suggestion_view_->SetProperty(views::kMarginsKey,
gfx::Insets(0, suggestion_indent, 0, 0));
suggestion_view_->OnMatchUpdate(this, match_);
keyword_view_->OnMatchUpdate(this, match_);
suggestion_tab_switch_button_->SetVisible(ShouldShowTabMatchButtonInline());
......@@ -250,7 +294,7 @@ void OmniboxResultView::SetMatch(const AutocompleteMatch& match) {
}
ApplyThemeAndRefreshIcons();
InvalidateLayout();
SetWidths();
}
void OmniboxResultView::ShowKeywordSlideAnimation(bool show_keyword) {
......@@ -324,7 +368,7 @@ void OmniboxResultView::ApplyThemeAndRefreshIcons(bool force_reapply_styles) {
if (OmniboxFieldTrial::IsRefinedFocusStateEnabled()) {
// The focus bar indicates when the suggestion is focused. Do not show the
// focus bar if an auxiliary button is selected.
focus_bar_->SetVisible(
selection_indicator_->SetVisible(
IsMatchSelected() &&
popup_contents_view_->model()->selected_line_state() ==
OmniboxPopupModel::NORMAL);
......@@ -414,78 +458,6 @@ void OmniboxResultView::ButtonPressed(OmniboxPopupModel::LineState state,
////////////////////////////////////////////////////////////////////////////////
// OmniboxResultView, views::View overrides:
void OmniboxResultView::Layout() {
views::View::Layout();
// NOTE: While animating the keyword match, both matches may be visible.
int suggestion_width = width();
if (keyword_view_->GetVisible()) {
const int max_kw_x =
suggestion_width - OmniboxMatchCellView::GetTextIndent();
suggestion_width =
keyword_slide_animation_->CurrentValueBetween(max_kw_x, 0);
keyword_view_->SetBounds(suggestion_width, 0, width() - suggestion_width,
height());
}
// Add buttons from right to left, shrinking the suggestion width as we go.
// To avoid clutter, don't show either button for matches with keyword.
// TODO(tommycli): We should probably use a layout manager here.
if (remove_suggestion_button_->GetVisible()) {
const gfx::Size button_size = remove_suggestion_button_->GetPreferredSize();
suggestion_width -=
button_size.width() + OmniboxMatchCellView::kMarginRight;
// Center the button vertically.
const int vertical_margin =
(suggestion_view_->height() - button_size.height()) / 2;
remove_suggestion_button_->SetBounds(suggestion_width, vertical_margin,
button_size.width(),
button_size.height());
}
if (ShouldShowTabMatchButtonInline()) {
suggestion_tab_switch_button_->ProvideWidthHint(suggestion_width);
const gfx::Size ts_button_size =
suggestion_tab_switch_button_->GetPreferredSize();
if (ts_button_size.width() > 0) {
suggestion_tab_switch_button_->SetSize(ts_button_size);
// Give the tab switch button a right margin matching the text.
suggestion_width -=
ts_button_size.width() + OmniboxMatchCellView::kMarginRight;
// Center the button vertically.
const int vertical_margin =
(suggestion_view_->height() - ts_button_size.height()) / 2;
suggestion_tab_switch_button_->SetPosition(
gfx::Point(suggestion_width, vertical_margin));
suggestion_tab_switch_button_->SetVisible(true);
} else {
suggestion_tab_switch_button_->SetVisible(false);
}
}
const int suggestion_indent =
popup_contents_view_->InExplicitExperimentalKeywordMode() ? 70 : 0;
const int suggestion_height = suggestion_view_->GetPreferredSize().height();
suggestion_view_->SetBounds(suggestion_indent, 0,
suggestion_width - suggestion_indent,
suggestion_height);
if (OmniboxFieldTrial::IsSuggestionButtonRowEnabled()) {
// TODO(orinj): Determine and use the best way to set bounds; probably
// GetPreferredSize() with a layout manager.
// Put it below the suggestion view.
button_row_->SetBounds(0, suggestion_height,
suggestion_width - suggestion_indent,
button_row_->GetPreferredSize().height());
}
if (OmniboxFieldTrial::IsRefinedFocusStateEnabled()) {
focus_bar_->SetBounds(0, 0, OmniboxResultFocusBar::kStrokeThickness,
suggestion_height);
}
}
bool OmniboxResultView::OnMousePressed(const ui::MouseEvent& event) {
if (event.IsOnlyLeftMouseButton())
popup_contents_view_->SetSelectedLineForMouseOrTouch(model_index_);
......@@ -578,17 +550,6 @@ void OmniboxResultView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
node_data->AddState(ax::mojom::State::kHovered);
}
gfx::Size OmniboxResultView::CalculatePreferredSize() const {
gfx::Size size = suggestion_view_->GetPreferredSize();
if (keyword_view_->GetVisible())
size.SetToMax(keyword_view_->GetPreferredSize());
if (OmniboxFieldTrial::IsSuggestionButtonRowEnabled() &&
button_row_->GetVisible()) {
// Add height of button row.
size.set_height(size.height() + button_row_->GetPreferredSize().height());
}
return size;
}
void OmniboxResultView::OnThemeChanged() {
views::View::OnThemeChanged();
......@@ -651,6 +612,18 @@ void OmniboxResultView::UpdateRemoveSuggestionVisibility() {
InvalidateLayout();
}
void OmniboxResultView::SetWidths() {
// TODO(pkasting): Use an animating layout manager
const int min_keyword_width =
std::min(OmniboxMatchCellView::GetTextIndent(), width());
keyword_view_->SetPreferredSize(
{keyword_slide_animation_->CurrentValueBetween(min_keyword_width,
width()),
keyword_view_->CalculatePreferredSize().height()});
InvalidateLayout();
}
////////////////////////////////////////////////////////////////////////////////
// OmniboxResultView, views::View overrides, private:
......@@ -661,12 +634,12 @@ const char* OmniboxResultView::GetClassName() const {
void OmniboxResultView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
keyword_slide_animation_->SetSlideDuration(
base::TimeDelta::FromMilliseconds(width() / 4));
InvalidateLayout();
SetWidths();
}
////////////////////////////////////////////////////////////////////////////////
// OmniboxResultView, views::AnimationDelegateViews overrides, private:
void OmniboxResultView::AnimationProgressed(const gfx::Animation* animation) {
InvalidateLayout();
SetWidths();
}
chrome/browser/ui/views/omnibox/omnibox_result_view.h
View file @ 6f06c4ad
......@@ -29,7 +29,7 @@ class OmniboxMatchCellView;
class OmniboxPopupContentsView;
class OmniboxSuggestionButtonRowView;
class OmniboxTabSwitchButton;
class OmniboxResultFocusBar;
class OmniboxResultSelectionIndicator;
enum class OmniboxPart;
enum class OmniboxPartState;
......@@ -96,14 +96,12 @@ class OmniboxResultView : public views::View,
void EmitTextChangedAccessiblityEvent();
// views::View:
void Layout() override;
bool OnMousePressed(const ui::MouseEvent& event) override;
bool OnMouseDragged(const ui::MouseEvent& event) override;
void OnMouseReleased(const ui::MouseEvent& event) override;
void OnMouseEntered(const ui::MouseEvent& event) override;
void OnMouseExited(const ui::MouseEvent& event) override;
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
gfx::Size CalculatePreferredSize() const override;
void OnThemeChanged() override;
private:
......@@ -127,6 +125,9 @@ class OmniboxResultView : public views::View,
// state.
void UpdateRemoveSuggestionVisibility();
// Sets the widths of the suggestion and keyword and calls Layout().
void SetWidths();
// views::View:
const char* GetClassName() const override;
void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
......@@ -149,23 +150,26 @@ class OmniboxResultView : public views::View,
// For sliding in the keyword search.
std::unique_ptr<gfx::SlideAnimation> keyword_slide_animation_;
// Container for the first row (for everything expect |button_row_|).
views::View* suggestion_container_;
// Weak pointers for easy reference.
OmniboxMatchCellView* suggestion_view_; // The leading (or left) view.
OmniboxMatchCellView* keyword_view_; // The trailing (or right) view.
OmniboxTabSwitchButton* suggestion_tab_switch_button_;
// The blue bar used to indicate focus. This is currently only used if
// The blue bar used to indicate selection. This is currently only used if
// omnibox-refined-focus-state flag is enabled.
OmniboxResultFocusBar* focus_bar_ = nullptr;
// The row of buttons, only assigned and used if OmniboxSuggestionButtonRow
// feature is enabled. It is owned by the base view, not this raw pointer.
OmniboxSuggestionButtonRowView* button_row_ = nullptr;
OmniboxResultSelectionIndicator* selection_indicator_ = nullptr;
// The "X" button at the end of the match cell, used to remove suggestions.
views::ImageButton* remove_suggestion_button_;
views::FocusRing* remove_suggestion_focus_ring_ = nullptr;
// The row of buttons, only assigned and used if OmniboxSuggestionButtonRow
// feature is enabled. It is owned by the base view, not this raw pointer.
OmniboxSuggestionButtonRowView* button_row_ = nullptr;
// Keeps track of mouse-enter and mouse-exit events of child Views.
OmniboxMouseEnterExitHandler mouse_enter_exit_handler_;
......
chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.cc
View file @ 6f06c4ad
......@@ -72,6 +72,17 @@ OmniboxTabSwitchButton::OmniboxTabSwitchButton(
OmniboxTabSwitchButton::~OmniboxTabSwitchButton() = default;
void OmniboxTabSwitchButton::OnBoundsChanged(const gfx::Rect& previous_bounds) {
MdTextButton::OnBoundsChanged(previous_bounds);
base::string16 text = hint_;
if (width() <= icon_only_width_)
text = base::string16();
else if (width() <= short_text_width_)
text = hint_short_;
SetText(text);
}
void OmniboxTabSwitchButton::StateChanged(ButtonState old_state) {
MdTextButton::StateChanged(old_state);
if (GetState() == STATE_NORMAL && old_state == STATE_PRESSED) {
......@@ -98,15 +109,33 @@ void OmniboxTabSwitchButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
IsSelected());
}
void OmniboxTabSwitchButton::UpdateBackground() {
focus_ring()->SchedulePaint();
// static
views::FlexRule OmniboxTabSwitchButton::GetFlexRule() {
// The rule below snaps between full, short, icon-only, and zero widths.
return base::BindRepeating(
[](const views::View* view, const views::SizeBounds& maximum_size) {
gfx::Size preferred_size = view->GetPreferredSize();
int width;
if (!maximum_size.width().is_bounded()) {
// Until width is bounded, return 0 to allocate flex excess correctly.
width = 0;
} else if (full_text_width_ <= maximum_size.width()) {
width = full_text_width_;
} else if (short_text_width_ <= maximum_size.width()) {
width = short_text_width_;
} else if (icon_only_width_ < maximum_size.width()) {
width = icon_only_width_;
} else {
// Available width is too small to fit even the icon only. So don't
// show button at all.
width = 0;
}
return gfx::Size(width, preferred_size.height());
});
}
void OmniboxTabSwitchButton::ProvideWidthHint(int parent_width) {
base::string16 text;
int preferred_width = CalculateGoalWidth(parent_width, &text);
SetText(text);
SetPreferredSize({preferred_width, GetPreferredSize().height()});
void OmniboxTabSwitchButton::UpdateBackground() {
focus_ring()->SchedulePaint();
}
bool OmniboxTabSwitchButton::IsSelected() const {
......@@ -115,17 +144,3 @@ bool OmniboxTabSwitchButton::IsSelected() const {
popup_contents_view_->model()->selected_line_state() ==
OmniboxPopupModel::FOCUSED_BUTTON_TAB_SWITCH;
}
int OmniboxTabSwitchButton::CalculateGoalWidth(int parent_width,
base::string16* goal_text) {
if (full_text_width_ * 5 <= parent_width) {
*goal_text = hint_;
return full_text_width_;
}
if (short_text_width_ * 5 <= parent_width) {
*goal_text = hint_short_;
return short_text_width_;
}
*goal_text = base::string16();
return (icon_only_width_ * 5 <= parent_width) ? icon_only_width_ : 0;
}
chrome/browser/ui/views/omnibox/omnibox_tab_switch_button.h
View file @ 6f06c4ad
......@@ -6,6 +6,7 @@
#define CHROME_BROWSER_UI_VIEWS_OMNIBOX_OMNIBOX_TAB_SWITCH_BUTTON_H_
#include "ui/views/controls/button/md_text_button.h"
#include "ui/views/layout/flex_layout_types.h"
class OmniboxPopupContentsView;
class OmniboxResultView;
......@@ -22,26 +23,22 @@ class OmniboxTabSwitchButton : public views::MdTextButton {
~OmniboxTabSwitchButton() override;
// views::MdTextButton:
void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
void StateChanged(ButtonState old_state) override;
void OnThemeChanged() override;
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
// Returns the FlexRule that should be used to size this button.
static views::FlexRule GetFlexRule();
// Called by parent views to change background on external (not mouse related)
// event (tab key).
void UpdateBackground();
// Called by parent view to provide the width of the surrounding area
// so the button can adjust its size or even presence.
void ProvideWidthHint(int width);
private:
// Consults the parent views to see if the button is selected.
bool IsSelected() const;
// Helper function to translate parent width into goal width, and
// pass back the text at that width.
int CalculateGoalWidth(int parent_width, base::string16* goal_text);
// The ancestor views.
OmniboxPopupContentsView* const popup_contents_view_;
OmniboxResultView* const result_view_;
......
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