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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 78803002: Fixing focus highlight on high DPI devices for accessibility (Closed)

Created:
7 years, 1 month ago by Mr4D (OOO till 08-26)
Modified:
7 years ago
CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org, kalyank, sadrul, ben+ash_chromium.org, tfarina
Visibility:
Public.

Description

Fixing focus highlight on high DPI devices for accessibility The name seems rather odd, but I couldn't come up with something better... BUG=320756 TEST=visual Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=238185

Patch Set 1 #

Patch Set 2 : Pulled drawing function into new FocusBorder class #

Patch Set 3 : merge & some offsets changed #

Patch Set 4 : found message center offset #

Total comments: 6

Patch Set 5 : Addressed #

Unified diffs Side-by-side diffs Delta from patch set Stats (+98 lines, -139 lines) Patch
M ash/shelf/shelf_view.cc View 1 2 2 chunks +3 lines, -17 lines 0 comments Download
M ash/system/tray/actionable_view.cc View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M ash/system/tray/tray_popup_header_button.h View 1 1 chunk +0 lines, -1 line 0 comments Download
M ash/system/tray/tray_popup_header_button.cc View 1 2 2 chunks +4 lines, -7 lines 0 comments Download
M ash/system/tray/tray_popup_label_button.h View 1 1 chunk +0 lines, -2 lines 0 comments Download
M ash/system/tray/tray_popup_label_button.cc View 1 2 1 chunk +3 lines, -7 lines 0 comments Download
M ui/app_list/views/folder_header_view.cc View 1 2 chunks +4 lines, -9 lines 0 comments Download
M ui/gfx/canvas.h View 1 1 chunk +4 lines, -0 lines 0 comments Download
M ui/gfx/canvas.cc View 1 2 1 chunk +16 lines, -0 lines 0 comments Download
M ui/message_center/message_center.gyp View 1 1 chunk +0 lines, -2 lines 0 comments Download
M ui/message_center/views/message_center_button_bar.cc View 1 2 2 chunks +4 lines, -8 lines 0 comments Download
D ui/message_center/views/message_center_focus_border.h View 1 1 chunk +0 lines, -30 lines 0 comments Download
M ui/message_center/views/message_center_focus_border.cc View 1 1 chunk +0 lines, -25 lines 0 comments Download
M ui/message_center/views/message_view.h View 1 2 1 chunk +0 lines, -1 line 0 comments Download
M ui/message_center/views/message_view.cc View 1 2 3 2 chunks +4 lines, -7 lines 0 comments Download
M ui/message_center/views/notification_view.cc View 1 2 3 chunks +3 lines, -8 lines 0 comments Download
M ui/message_center/views/notifier_settings_view.cc View 1 2 4 chunks +7 lines, -3 lines 0 comments Download
M ui/message_center/views/padded_button.h View 1 2 1 chunk +0 lines, -1 line 0 comments Download
M ui/message_center/views/padded_button.cc View 1 2 2 chunks +3 lines, -7 lines 0 comments Download
M ui/views/controls/slider.cc View 1 1 chunk +3 lines, -2 lines 0 comments Download
M ui/views/focus_border.h View 1 2 3 4 2 chunks +7 lines, -0 lines 0 comments Download
M ui/views/focus_border.cc View 1 2 3 4 3 chunks +31 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Mr4D (OOO till 08-26)
Please have a look!
7 years, 1 month ago (2013-11-20 16:32:19 UTC) #1
oshima
can't we just use views::FocusableBorder instead?
7 years, 1 month ago (2013-11-20 17:49:28 UTC) #2
Mr4D (OOO till 08-26)
As discussed - I consolidated all calls into the the FocusBorder class.
7 years ago (2013-11-25 14:55:04 UTC) #3
oshima
lgtm++ with nits. thank you! https://codereview.chromium.org/78803002/diff/90001/ui/views/focus_border.cc File ui/views/focus_border.cc (right): https://codereview.chromium.org/78803002/diff/90001/ui/views/focus_border.cc#newcode43 ui/views/focus_border.cc:43: SkColor focus_color, arguments can ...
7 years ago (2013-11-25 19:22:26 UTC) #4
Mr4D (OOO till 08-26)
Ben, can you do the honors of doing the owners review? Thanks! https://codereview.chromium.org/78803002/diff/90001/ui/views/focus_border.cc File ui/views/focus_border.cc ...
7 years ago (2013-11-25 19:35:13 UTC) #5
Ben Goodger (Google)
lgtm
7 years ago (2013-12-02 18:43:33 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/skuhne@chromium.org/78803002/110001
7 years ago (2013-12-02 18:46:34 UTC) #7
commit-bot: I haz the power
7 years ago (2013-12-02 21:07:33 UTC) #8
Message was sent while issue was closed. 
Change committed as 238185

Powered by Google App Engine
This is Rietveld 408576698