Commit c1aab56c authored by Tommy Li's avatar Tommy Li Committed by Commit Bot

[omnibox] Whole header row expands and collapses section (on Desktop).

Previously, the user had to click on the header button to expand or
collapse the section.

This CL makes it so they can click anywhere on the row.

This only affects the Views omnibox, and the NTP realbox, both on
Desktop.

Android already works like this, so it needs no further work.

Bug: 1113341c
Change-Id: I64de6db54ee63de3d428de69aa23c2759effe1cf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2340266
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: default avatarMoe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795713}
parent 7e05f84a
...@@ -64,14 +64,15 @@ ...@@ -64,14 +64,15 @@
<template is="dom-if" if="[[groupHasHeader_(groupId)]]"> <template is="dom-if" if="[[groupHasHeader_(groupId)]]">
<!-- Header cannot be tabbed into but gets focus when clicked. This stops <!-- Header cannot be tabbed into but gets focus when clicked. This stops
the dropdown from losing focus and closing as a result. --> the dropdown from losing focus and closing as a result. -->
<div class="header" tabindex="-1" on-focusin="onHeaderFocusin_" <div class="header" data-id$="[[groupId]]" tabindex="-1"
on-focusin="onHeaderFocusin_" on-click="onHeaderClick_"
aria-hidden="true" aria-hidden="true"
group-is-hidden$="[[groupIsHidden_(groupId, hiddenGroupIds_.*)]]"> group-is-hidden$="[[groupIsHidden_(groupId, hiddenGroupIds_.*)]]">
<span class="text">[[headerForGroup_(groupId)]]</span> <span class="text">[[headerForGroup_(groupId)]]</span>
<ntp-realbox-button data-id$="[[groupId]]" tabindex="0" role="button" <ntp-realbox-button tabindex="0" role="button"
title="[[toggleButtonTitleForGroup_(groupId, hiddenGroupIds_.*)]]" title="[[toggleButtonTitleForGroup_(groupId, hiddenGroupIds_.*)]]"
aria-label$="[[toggleButtonA11yLabelForGroup_(groupId, hiddenGroupIds_.*)]]" aria-label$="[[toggleButtonA11yLabelForGroup_(groupId, hiddenGroupIds_.*)]]"
on-click="onToggleButtonClick_" on-keydown="onToggleButtonKeydown_"> on-keydown="onToggleButtonKeydown_">
</ntp-realbox-button> </ntp-realbox-button>
</div> </div>
</template> </template>
......
...@@ -285,8 +285,8 @@ class RealboxDropdownElement extends PolymerElement { ...@@ -285,8 +285,8 @@ class RealboxDropdownElement extends PolymerElement {
* @param {!Event} e * @param {!Event} e
* @private * @private
*/ */
onToggleButtonClick_(e) { onHeaderClick_(e) {
const groupId = e.target.dataset.id; const groupId = e.currentTarget.dataset.id;
// Tell the backend to toggle visibility of the given suggestion group ID. // Tell the backend to toggle visibility of the given suggestion group ID.
this.pageHandler_.toggleSuggestionGroupIdVisibility(groupId); this.pageHandler_.toggleSuggestionGroupIdVisibility(groupId);
...@@ -309,7 +309,7 @@ class RealboxDropdownElement extends PolymerElement { ...@@ -309,7 +309,7 @@ class RealboxDropdownElement extends PolymerElement {
return; return;
} }
// Simulate a click so that it gets handled by |onToggleButtonClick_|. // Simulate a click so that it gets handled by |onHeaderClick_|.
e.target.click(); e.target.click();
e.preventDefault(); // Prevents default browser action. e.preventDefault(); // Prevents default browser action.
} }
......
...@@ -57,10 +57,7 @@ class OmniboxRowView::HeaderView : public views::View, ...@@ -57,10 +57,7 @@ class OmniboxRowView::HeaderView : public views::View,
views::FocusRing::Install(header_toggle_button_); views::FocusRing::Install(header_toggle_button_);
header_toggle_button_focus_ring_->SetHasFocusPredicate([&](View* view) { header_toggle_button_focus_ring_->SetHasFocusPredicate([&](View* view) {
return view->GetVisible() && return view->GetVisible() &&
row_view_->popup_model_->selection() == row_view_->popup_model_->selection() == HeaderSelection();
OmniboxPopupModel::Selection(
row_view_->line_,
OmniboxPopupModel::FOCUSED_BUTTON_HEADER);
}); });
if (row_view_->pref_service_) { if (row_view_->pref_service_) {
...@@ -107,6 +104,13 @@ class OmniboxRowView::HeaderView : public views::View, ...@@ -107,6 +104,13 @@ class OmniboxRowView::HeaderView : public views::View,
return gfx::Insets(vertical, left_inset, vertical, return gfx::Insets(vertical, left_inset, vertical,
OmniboxMatchCellView::kMarginRight); OmniboxMatchCellView::kMarginRight);
} }
bool OnMousePressed(const ui::MouseEvent& event) override {
// Needed to receive the OnMouseReleased event.
return true;
}
void OnMouseReleased(const ui::MouseEvent& event) override {
row_view_->popup_model_->TriggerSelectionAction(HeaderSelection());
}
void OnMouseEntered(const ui::MouseEvent& event) override { UpdateUI(); } void OnMouseEntered(const ui::MouseEvent& event) override { UpdateUI(); }
void OnMouseExited(const ui::MouseEvent& event) override { UpdateUI(); } void OnMouseExited(const ui::MouseEvent& event) override { UpdateUI(); }
void OnThemeChanged() override { void OnThemeChanged() override {
...@@ -129,18 +133,14 @@ class OmniboxRowView::HeaderView : public views::View, ...@@ -129,18 +133,14 @@ class OmniboxRowView::HeaderView : public views::View,
// views::ButtonListener: // views::ButtonListener:
void ButtonPressed(views::Button* sender, const ui::Event& event) override { void ButtonPressed(views::Button* sender, const ui::Event& event) override {
DCHECK_EQ(sender, header_toggle_button_); DCHECK_EQ(sender, header_toggle_button_);
row_view_->popup_model_->TriggerSelectionAction( row_view_->popup_model_->TriggerSelectionAction(HeaderSelection());
OmniboxPopupModel::Selection(row_view_->line_,
OmniboxPopupModel::FOCUSED_BUTTON_HEADER));
// The PrefChangeRegistrar will update the actual button toggle state. // The PrefChangeRegistrar will update the actual button toggle state.
} }
// Updates the UI state for the new hover or selection state. // Updates the UI state for the new hover or selection state.
void UpdateUI() { void UpdateUI() {
OmniboxPartState part_state = OmniboxPartState::NORMAL; OmniboxPartState part_state = OmniboxPartState::NORMAL;
if (row_view_->popup_model_->selection() == if (row_view_->popup_model_->selection() == HeaderSelection()) {
OmniboxPopupModel::Selection(
row_view_->line_, OmniboxPopupModel::FOCUSED_BUTTON_HEADER)) {
part_state = OmniboxPartState::SELECTED; part_state = OmniboxPartState::SELECTED;
} else if (IsMouseHovered()) { } else if (IsMouseHovered()) {
part_state = OmniboxPartState::HOVERED; part_state = OmniboxPartState::HOVERED;
...@@ -211,6 +211,12 @@ class OmniboxRowView::HeaderView : public views::View, ...@@ -211,6 +211,12 @@ class OmniboxRowView::HeaderView : public views::View,
header_toggle_button_->SetToggled(suggestion_group_hidden_); header_toggle_button_->SetToggled(suggestion_group_hidden_);
} }
// Convenience method to get the OmniboxPopupModel::Selection for this view.
OmniboxPopupModel::Selection HeaderSelection() {
return OmniboxPopupModel::Selection(
row_view_->line_, OmniboxPopupModel::FOCUSED_BUTTON_HEADER);
}
// Non-owning pointer our parent row view. We access a lot of private members // Non-owning pointer our parent row view. We access a lot of private members
// of our outer class. This lets us save quite a bit of state duplication. // of our outer class. This lets us save quite a bit of state duplication.
OmniboxRowView* const row_view_; OmniboxRowView* const row_view_;
......
...@@ -2032,6 +2032,21 @@ suite('NewTabPageRealboxTest', () => { ...@@ -2032,6 +2032,21 @@ suite('NewTabPageRealboxTest', () => {
matchEls = matchEls =
realbox.$.matches.shadowRoot.querySelectorAll('ntp-realbox-match'); realbox.$.matches.shadowRoot.querySelectorAll('ntp-realbox-match');
assertEquals(1, matchEls.length); assertEquals(1, matchEls.length);
testProxy.handler.reset();
// Show the second match by clicking the header.
headerEl.click();
await testProxy.handler.whenCalled('toggleSuggestionGroupIdVisibility')
.then((args) => {
assertEquals('100', args.suggestionGroupId);
});
assertEquals(
1, testProxy.handler.getCallCount('toggleSuggestionGroupIdVisibility'));
// Second match is visible again.
matchEls =
realbox.$.matches.shadowRoot.querySelectorAll('ntp-realbox-match');
assertEquals(2, matchEls.length);
}); });
test( test(
......
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