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

Issue 28012: Fullscreen mode UI.... (Closed)

Created:
11 years, 10 months ago by Peter Kasting
Modified:
9 years, 7 months ago
Reviewers:
Glen Murphy
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Fullscreen mode UI. This requires a change that is currently out for review from sky. BUG=534 TEST=App menu should have a "full screen mode" entry; going into fullscreen mode should display a bubble at the top of the screen, which should slide away after a few seconds; mousing to the screen top will show the bubble again, and clicking the link inside should exit fullscreen mode. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=10214

Patch Set 1 #

Total comments: 2

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+340 lines, -1 line) Patch
M chrome/app/generated_resources.grd View 1 3 chunks +12 lines, -0 lines 0 comments Download
M chrome/browser/views/browser_views.vcproj View 1 1 chunk +8 lines, -0 lines 0 comments Download
M chrome/browser/views/frame/browser_view.h View 1 2 chunks +3 lines, -0 lines 0 comments Download
M chrome/browser/views/frame/browser_view.cc View 1 3 chunks +8 lines, -1 line 0 comments Download
A chrome/browser/views/fullscreen_exit_bubble.h View 1 chunk +85 lines, -0 lines 0 comments Download
A chrome/browser/views/fullscreen_exit_bubble.cc View 1 1 chunk +219 lines, -0 lines 0 comments Download
M chrome/browser/views/toolbar_view.cc View 1 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/views/accelerator.cc View 1 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Peter Kasting
11 years, 10 months ago (2009-02-21 00:11:29 UTC) #1
Glen Murphy
11 years, 10 months ago (2009-02-23 20:40:33 UTC) #2
LGTM, with ignorable nits:

http://codereview.chromium.org/28012/diff/1/2
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/28012/diff/1/2#newcode563
Line 563: &Full screen mode
'Full screen' without the 'mode' is probably enough here.

http://codereview.chromium.org/28012/diff/1/5
File chrome/browser/views/fullscreen_exit_bubble.cc (right):

http://codereview.chromium.org/28012/diff/1/5#newcode47
Line 47: AddChildView(&link_);
I know it makes little difference, but I generally prefer things to be added
after they have their properties set (in case the add ever does something with
the props). Up to you though.

Powered by Google App Engine
This is Rietveld 408576698