Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1095)

Issue 2929803002: Make close button visible when focusing inline actions.

Created:
3 years, 6 months ago by fukino
Modified:
3 years, 6 months ago
Reviewers:
yoshiki
CC:
chromium-reviews, Peter Beverloo, mlamouri+watch-notifications_chromium.org, awdf+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Make close button visible when focusing inline actions. When inline action button is focused by keyboard, the close button becomes invisible. That's why we can't focus the close button by hitting TAB key repeatedly. This CL fixes it by making control buttons (including close button) visible when the notification or its descendant views have focus. BUG=731074 TEST=manually tested

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -4 lines) Patch
M ui/message_center/views/notification_view.h View 1 chunk +3 lines, -0 lines 0 comments Download
M ui/message_center/views/notification_view.cc View 2 chunks +6 lines, -4 lines 0 comments Download

Messages

Total messages: 4 (1 generated)
fukino
yoshiki@, could you take a look?
3 years, 6 months ago (2017-06-08 12:51:33 UTC) #2
yoshiki
At least on ARC notification, the close button shouldn't be visible when the focus is ...
3 years, 6 months ago (2017-06-09 08:32:35 UTC) #3
fukino
3 years, 6 months ago (2017-06-09 08:50:34 UTC) #4
On 2017/06/09 08:32:35, yoshiki wrote:
> At least on ARC notification, the close button shouldn't be visible when the
> focus is on notification except for buttons, because some elements on
> notification might be placed under the buttons.
> 
> Should we keep the consistency with ARC notifications?

I got it.
When the inline actions are focused, the close button should be invisible but
need to be focused.
Using alpha instead if SetVisible() might interfere the clickable area on the
header, so I'll look for another approach.

Powered by Google App Engine
This is Rietveld 408576698