Chromium Code Reviews
Help | Chromium Project | Sign in
(2)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 1 month ago by Peter Kasting
Modified:
5 years, 10 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
Trybot results:
Commit queue not available (can’t edit this change).

Messages

Total messages: 2 (0 generated)
Peter Kasting
8 years, 1 month ago (2009-02-21 00:11:29 UTC) #1
Glen Murphy
8 years, 1 month 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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld d1a128a62