Commit f16582bf authored by yoshiki iguchi's avatar yoshiki iguchi Committed by Commit Bot

Fix focus order and button visibility on notification

This CL fixes the wrong reverse focus order of notification (see the issue for detail). Previously the close and settings buttons are skipped in reverse order.

This also fixes the visibility of close and settings button. These buttons should be visible only when they have a focus. Previously they are visible even when the focus is not on these buttons but on the notification.

BUG=739293
TEST=manual test

Change-Id: I8af330314410bf4b1a26be51d86cee312b4344ab
Reviewed-on: https://chromium-review.googlesource.com/571522Reviewed-by: default avatarYuichiro Hanada <yhanada@chromium.org>
Commit-Queue: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486719}
parent 7d0d09ec
......@@ -278,9 +278,6 @@ void ArcNotificationContentView::MaybeCreateFloatingControlButtons() {
// a horizontal box.
control_buttons_view_ =
new message_center::NotificationControlButtonsView(notification_view);
control_buttons_view_->SetPaintToLayer(ui::LAYER_TEXTURED);
control_buttons_view_->layer()->SetFillsBoundsOpaquely(false);
control_buttons_view_->SetBackgroundColor(
GetControlButtonBackgroundColor(item_->GetShownContents()));
control_buttons_view_->ShowSettingsButton(
......
......@@ -40,6 +40,11 @@ NotificationControlButtonsView::NotificationControlButtonsView(
bgcolor_target_(kInitialBackgroundColor) {
DCHECK(message_view);
SetLayoutManager(new views::BoxLayout(views::BoxLayout::kHorizontal));
// Use layer to change the opacity.
SetPaintToLayer();
layer()->SetFillsBoundsOpaquely(false);
SetBackground(views::CreateSolidBackground(kInitialBackgroundColor));
}
......@@ -91,6 +96,7 @@ void NotificationControlButtonsView::ShowSettingsButton(bool show) {
void NotificationControlButtonsView::SetBackgroundColor(
const SkColor& target_bgcolor) {
DCHECK(background());
if (background()->get_color() != target_bgcolor) {
bgcolor_origin_ = background()->get_color();
bgcolor_target_ = target_bgcolor;
......@@ -103,6 +109,14 @@ void NotificationControlButtonsView::SetBackgroundColor(
}
}
void NotificationControlButtonsView::SetVisible(bool visible) {
DCHECK(layer());
// Manipulate the opacity instead of changing the visibility to keep the tab
// order even when the view is invisible.
layer()->SetOpacity(visible ? 1. : 0.);
set_can_process_events_within_subtree(visible);
}
void NotificationControlButtonsView::RequestFocusOnCloseButton() {
if (close_button_)
close_button_->RequestFocus();
......
......@@ -60,6 +60,7 @@ class MESSAGE_CENTER_EXPORT NotificationControlButtonsView
// views::View
const char* GetClassName() const override;
void SetVisible(bool visible) override;
// views::ButtonListener
void ButtonPressed(views::Button* sender, const ui::Event& event) override;
......
......@@ -637,8 +637,7 @@ void NotificationView::UpdateControlButtonsVisibility() {
// (1) the mouse is hovering on the notification.
// (2) the focus is on the control buttons.
const bool target_visibility =
IsMouseHovered() || HasFocus() ||
control_buttons_view_->IsCloseButtonFocused() ||
IsMouseHovered() || control_buttons_view_->IsCloseButtonFocused() ||
control_buttons_view_->IsSettingsButtonFocused();
#else
// On non Chrome OS, the settings button and the close button are always
......
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