Issue 3156049: First set of changes for M7 NTP.

Issue 3156049: First set of changes for M7 NTP. (Closed)

Created:
10 years, 4 months ago by Aaron Boodman

Modified:
9 years, 6 months ago

Reviewers:
arv (Not doing code reviews), Miranda Callahan

CC:
chromium-reviews, ben+cc_chromium.org, Glen Murphy

Base URL:
git://codf21.jail/chromium.git

Visibility:
Public.

Description

First set of changes for M7 NTP. This doesn't do any behavior changes -- it just implements the styling. So we still have multi expand, and the recently closed section is still toggleable. I also didn't make the changes to the placement or text of the web store icon. I will make those separately. Screen caps: http://aaronboodman.com/z_dropbox/cr3156049/ BUG=53248 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=57473

Patch Set 1 #

Patch Set 2 : pre-review cleanup #

Total comments: 6

Created: 10 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+106 lines, -127 lines)			Patch
M	chrome/app/generated_resources.grd	View		1 chunk	+4 lines, -1 line	0 comments	Download
A	chrome/app/theme/ntp_header_background.png	View		Binary file		0 comments	Download
A	chrome/app/theme/ntp_header_background_active.png	View		Binary file		0 comments	Download
A	chrome/app/theme/ntp_header_background_hover.png	View		Binary file		0 comments	Download
M	chrome/app/theme/theme_resources.grd	View		1 chunk	+3 lines, -0 lines	0 comments	Download
M	chrome/browser/browser_theme_provider.h	View		1 chunk	+2 lines, -0 lines	0 comments	Download
M	chrome/browser/browser_theme_provider.cc	View		2 chunks	+5 lines, -0 lines	0 comments	Download
M	chrome/browser/dom_ui/new_tab_ui.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/dom_ui/ntp_resource_cache.cc	View		5 chunks	+14 lines, -5 lines	0 comments	Download
M	chrome/browser/dom_ui/shown_sections_handler.h	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/browser/dom_ui/shown_sections_handler.cc	View		2 chunks	+7 lines, -1 line	1 comment	Download
M	chrome/browser/resources/new_new_tab.css	View	1	9 chunks	+25 lines, -56 lines	4 comments	Download
M	chrome/browser/resources/new_new_tab.html	View	1	5 chunks	+12 lines, -21 lines	0 comments	Download
M	chrome/browser/resources/new_new_tab.js	View	1	3 chunks	+13 lines, -3 lines	0 comments	Download
M	chrome/browser/resources/new_tab_theme.css	View	1	3 chunks	+12 lines, -21 lines	0 comments	Download
M	chrome/browser/resources/ntp/apps.css	View		1 chunk	+2 lines, -1 line	1 comment	Download
M	chrome/browser/resources/ntp/apps.js	View	1	1 chunk	+4 lines, -16 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Miranda Callahan

LGTM, w/question about comment phrasing. http://codereview.chromium.org/3156049/diff/5001/6010 File chrome/browser/dom_ui/shown_sections_handler.cc (right): http://codereview.chromium.org/3156049/diff/5001/6010#newcode117 chrome/browser/dom_ui/shown_sections_handler.cc:117: // We reset people ...

10 years, 4 months ago (2010-08-26 02:57:07 UTC) #2

arv (Not doing code reviews)

10 years, 3 months ago (2010-08-31 20:33:57 UTC) #3

FYI

http://codereview.chromium.org/3156049/diff/5001/6012
File chrome/browser/resources/new_new_tab.css (right):

http://codereview.chromium.org/3156049/diff/5001/6012#newcode23
chrome/browser/resources/new_new_tab.css:23: padding:10px 0 20px;
whitespace

http://codereview.chromium.org/3156049/diff/5001/6012#newcode380
chrome/browser/resources/new_new_tab.css:380: .section > h2 > img {
RTL?

-webkit-margin-start: -13px;
-webkit-padding-end: 4px;

http://codereview.chromium.org/3156049/diff/5001/6012#newcode386
chrome/browser/resources/new_new_tab.css:386: -webkit-transform:rotate(90deg);
ws after :

Can you add a transition for this transform?

http://codereview.chromium.org/3156049/diff/5001/6012#newcode390
chrome/browser/resources/new_new_tab.css:390: padding-right: 4px;
rtl

http://codereview.chromium.org/3156049/diff/5001/6016
File chrome/browser/resources/ntp/apps.css (right):

http://codereview.chromium.org/3156049/diff/5001/6016#newcode38
chrome/browser/resources/ntp/apps.css:38: font-family: Helvetica, Arial;
In general it is better to only set the font-family at the body and let all
elements inherit it

Expand Messages | Collapse Messages