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

Issue 10836227: Fix status area accessability issues. (Closed)

Created:
8 years, 4 months ago by stevenjb
Modified:
8 years, 4 months ago
Reviewers:
sadrul
CC:
chromium-reviews, sadrul, ben+watch_chromium.org
Visibility:
Public.

Description

Fix status area accessability issues. See BUG for description of issues addressed here. Primarily this moves a bunch of code from SystemTray inot TrayBackgroundView so that it can be shared by WebNotificationTray, but there is a bit of cleanup and a few bug fixes as well, primarily with how the tray view is hilighted, the child ordering of the trays (so that the system tray is first and thus initially focused), and correctly preventing the launcher from auto-hiding. BUG=142506

Patch Set 1 #

Total comments: 6

Patch Set 2 : Address feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+235 lines, -141 lines) Patch
M ash/ash_strings.grd View 1 1 chunk +3 lines, -0 lines 0 comments Download
M ash/system/status_area_widget.h View 3 chunks +6 lines, -0 lines 0 comments Download
M ash/system/status_area_widget.cc View 1 6 chunks +32 lines, -9 lines 0 comments Download
M ash/system/status_area_widget_delegate.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M ash/system/tray/system_tray.h View 1 6 chunks +8 lines, -25 lines 0 comments Download
M ash/system/tray/system_tray.cc View 1 12 chunks +31 lines, -94 lines 0 comments Download
M ash/system/tray/system_tray_bubble.h View 1 chunk +1 line, -0 lines 0 comments Download
M ash/system/tray/system_tray_bubble.cc View 1 2 chunks +6 lines, -0 lines 0 comments Download
M ash/system/tray/tray_background_view.h View 1 6 chunks +22 lines, -3 lines 0 comments Download
M ash/system/tray/tray_background_view.cc View 1 5 chunks +66 lines, -3 lines 0 comments Download
M ash/system/tray/tray_bubble_view.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M ash/system/tray/tray_bubble_view.cc View 1 1 chunk +1 line, -2 lines 0 comments Download
M ash/system/web_notification/web_notification_tray.h View 1 1 chunk +8 lines, -0 lines 0 comments Download
M ash/system/web_notification/web_notification_tray.cc View 1 10 chunks +45 lines, -1 line 0 comments Download
M ash/wm/shelf_layout_manager.cc View 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 4 (0 generated)
stevenjb
8 years, 4 months ago (2012-08-14 00:08:12 UTC) #1
stevenjb
Ping? (This is ready for review)
8 years, 4 months ago (2012-08-15 16:46:48 UTC) #2
sadrul
Refactoring is best done in separate CLs. http://codereview.chromium.org/10836227/diff/1/ash/system/status_area_widget.cc File ash/system/status_area_widget.cc (right): http://codereview.chromium.org/10836227/diff/1/ash/system/status_area_widget.cc#newcode308 ash/system/status_area_widget.cc:308: system_tray_->CreateItems(); Maybe ...
8 years, 4 months ago (2012-08-15 16:47:01 UTC) #3
stevenjb
8 years, 4 months ago (2012-08-15 21:51:04 UTC) #4
Yeah, sorry for the complex CL, broken into three parts now.

http://codereview.chromium.org/10836227/diff/1/ash/system/status_area_widget.cc
File ash/system/status_area_widget.cc (right):

http://codereview.chromium.org/10836227/diff/1/ash/system/status_area_widget....
ash/system/status_area_widget.cc:308: system_tray_->CreateItems();
On 2012/08/15 16:47:01, sadrul wrote:
> Maybe CreateItems can be called from Initialize now?

Done.

http://codereview.chromium.org/10836227/diff/1/ash/system/status_area_widget....
ash/system/status_area_widget.cc:314:
UpdateAfterLoginStatusChange(system_tray_delegate_->GetUserLoginStatus());
On 2012/08/15 16:47:01, sadrul wrote:
> Can system_tray_delegate() be NULL here?
Looks like not, removed test.

http://codereview.chromium.org/10836227/diff/1/ash/system/tray/system_tray_bu...
File ash/system/tray/system_tray_bubble.cc (right):

http://codereview.chromium.org/10836227/diff/1/ash/system/tray/system_tray_bu...
ash/system/tray/system_tray_bubble.cc:328: tray_->UpdateShouldShowLauncher();
On 2012/08/15 16:47:01, sadrul wrote:
> Will this hide the launcher immediately?

This will trigger a call to
Shell::GetInstance()->shelf()->UpdateAutoHideState();

Powered by Google App Engine
This is Rietveld 408576698