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

Issue 12038090: Layout the NTP footer with flexboxes. (Closed)

Created:
7 years, 11 months ago by dconnelly
Modified:
7 years, 10 months ago
CC:
chromium-reviews, dbeam+watch-ntp_chromium.org, estade+watch_chromium.org, arv (Not doing code reviews), pedrosimonetti+watch_chromium.org
Visibility:
Public.

Description

Layout the NTP footer with flexboxes. This updates the CSS flexbox property names to their most recent iterations, displays the entire footer in a flexbox to make the layout more obvious, and rearranges the footer's right-side flexbox so that it always has the same width. The logo on the footer's left side is resized once when the page is rendered to make it the same width as the right side. This keeps the nav dots centered in the footer. BUG= Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=182190

Patch Set 1 #

Total comments: 21

Patch Set 2 : Clean up CSS per review suggestions #

Patch Set 3 : classList#toggle #

Total comments: 2

Patch Set 4 : move separator out of webstore link #

Total comments: 3

Patch Set 5 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+50 lines, -19 lines) Patch
M chrome/browser/resources/ntp4/new_tab.css View 1 2 3 5 chunks +30 lines, -8 lines 0 comments Download
M chrome/browser/resources/ntp4/new_tab.html View 1 2 3 1 chunk +6 lines, -7 lines 0 comments Download
M chrome/browser/resources/ntp4/new_tab.js View 1 2 chunks +11 lines, -0 lines 0 comments Download
M chrome/browser/resources/ntp4/other_sessions.js View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/resources/ntp4/recently_closed.js View 1 2 2 chunks +1 line, -2 lines 0 comments Download

Messages

Total messages: 36 (0 generated)
dconnelly
7 years, 11 months ago (2013-01-25 13:03:29 UTC) #1
dconnelly
Hi James, I had Evan as a reviewer on a couple of CLs, but it ...
7 years, 11 months ago (2013-01-25 15:57:40 UTC) #2
James Hawkins
On 2013/01/25 15:57:40, dconnelly wrote: > Hi James, > > I had Evan as a ...
7 years, 11 months ago (2013-01-25 17:14:45 UTC) #3
dconnelly1
By out of office, I mean that he's on vacation auto-responder that says "return date ...
7 years, 11 months ago (2013-01-25 17:26:05 UTC) #4
Dan Beam
so we already did this once or twice (used new display: [-webkit-]flex;) and it kept ...
7 years, 11 months ago (2013-01-25 20:15:50 UTC) #5
Dan Beam
for example: https://chromiumcodereview.appspot.com/10332274
7 years, 11 months ago (2013-01-25 20:17:53 UTC) #6
Patrick Dubroy
On 2013/01/25 20:17:53, Dan Beam wrote: > for example: https://chromiumcodereview.appspot.com/10332274 The flexbox standard has moved ...
7 years, 10 months ago (2013-01-28 08:14:23 UTC) #7
dconnelly1
So, given this information, does anybody feel like doing a review? On Mon, Jan 28, ...
7 years, 10 months ago (2013-01-28 21:32:17 UTC) #8
Dan Beam
yes, I can review. have you been testing the keyboard tab order while developing this, ...
7 years, 10 months ago (2013-01-28 21:46:48 UTC) #9
dconnelly1
Good news! The keyboard tab order appears to be working properly. I can sign in ...
7 years, 10 months ago (2013-01-29 14:37:26 UTC) #10
dconnelly1
Ping On Tue, Jan 29, 2013 at 3:37 PM, Daniel Connelly <dconnelly@google.com> wrote: > Good ...
7 years, 10 months ago (2013-01-31 08:34:40 UTC) #11
Dan Beam
sorry, was in MTV, looking now
7 years, 10 months ago (2013-01-31 19:24:56 UTC) #12
Dan Beam
https://chromiumcodereview.appspot.com/12038090/diff/1/chrome/browser/resources/ntp4/new_tab.css File chrome/browser/resources/ntp4/new_tab.css (right): https://chromiumcodereview.appspot.com/12038090/diff/1/chrome/browser/resources/ntp4/new_tab.css#newcode184 chrome/browser/resources/ntp4/new_tab.css:184: -webkit-flex-direction: row; this seems to be the default value ...
7 years, 10 months ago (2013-01-31 20:22:10 UTC) #13
dconnelly
I've responded to some of your comments, and I'm working on the others. https://chromiumcodereview.appspot.com/12038090/diff/1/chrome/browser/resources/ntp4/new_tab.css File ...
7 years, 10 months ago (2013-02-01 14:34:14 UTC) #14
dconnelly
https://chromiumcodereview.appspot.com/12038090/diff/1/chrome/browser/resources/ntp4/new_tab.css File chrome/browser/resources/ntp4/new_tab.css (right): https://chromiumcodereview.appspot.com/12038090/diff/1/chrome/browser/resources/ntp4/new_tab.css#newcode184 chrome/browser/resources/ntp4/new_tab.css:184: -webkit-flex-direction: row; On 2013/01/31 20:22:11, Dan Beam wrote: > ...
7 years, 10 months ago (2013-02-04 16:40:45 UTC) #15
Dan Beam
https://chromiumcodereview.appspot.com/12038090/diff/1/chrome/browser/resources/ntp4/other_sessions.js File chrome/browser/resources/ntp4/other_sessions.js (right): https://chromiumcodereview.appspot.com/12038090/diff/1/chrome/browser/resources/ntp4/other_sessions.js#newcode248 chrome/browser/resources/ntp4/other_sessions.js:248: if (isTabSyncEnabled) On 2013/02/04 16:40:45, dconnelly wrote: > This ...
7 years, 10 months ago (2013-02-04 19:01:10 UTC) #16
dconnelly
On 2013/02/04 19:01:10, Dan Beam wrote: > https://chromiumcodereview.appspot.com/12038090/diff/1/chrome/browser/resources/ntp4/other_sessions.js > File chrome/browser/resources/ntp4/other_sessions.js (right): > > https://chromiumcodereview.appspot.com/12038090/diff/1/chrome/browser/resources/ntp4/other_sessions.js#newcode248 ...
7 years, 10 months ago (2013-02-05 09:01:49 UTC) #17
Dan Beam
wouldn't laying stuff out like this (2 evenly flexed nodes on both sides) make sure ...
7 years, 10 months ago (2013-02-05 20:10:46 UTC) #18
Dan Beam
this would avoid the need for JS
7 years, 10 months ago (2013-02-05 20:10:58 UTC) #19
Patrick Dubroy
On 2013/02/05 20:10:46, Dan Beam wrote: > wouldn't laying stuff out like this (2 evenly ...
7 years, 10 months ago (2013-02-05 20:57:29 UTC) #20
Dan Beam
considering estade@ is an OWNER, there's no bug ######, and he's the one that knows ...
7 years, 10 months ago (2013-02-05 22:19:03 UTC) #21
Evan Stade
looks pretty much ok to me https://chromiumcodereview.appspot.com/12038090/diff/20001/chrome/browser/resources/ntp4/new_tab.html File chrome/browser/resources/ntp4/new_tab.html (right): https://chromiumcodereview.appspot.com/12038090/diff/20001/chrome/browser/resources/ntp4/new_tab.html#newcode119 chrome/browser/resources/ntp4/new_tab.html:119: <div id="vertical-separator"></div> seems ...
7 years, 10 months ago (2013-02-05 23:26:47 UTC) #22
dconnelly
On 2013/02/05 22:19:03, Dan Beam wrote: > considering estade@ is an OWNER, there's no bug ...
7 years, 10 months ago (2013-02-06 14:58:29 UTC) #23
dconnelly
https://chromiumcodereview.appspot.com/12038090/diff/20001/chrome/browser/resources/ntp4/new_tab.html File chrome/browser/resources/ntp4/new_tab.html (right): https://chromiumcodereview.appspot.com/12038090/diff/20001/chrome/browser/resources/ntp4/new_tab.html#newcode119 chrome/browser/resources/ntp4/new_tab.html:119: <div id="vertical-separator"></div> On 2013/02/05 23:26:47, Evan Stade wrote: > ...
7 years, 10 months ago (2013-02-06 17:15:10 UTC) #24
Evan Stade
https://chromiumcodereview.appspot.com/12038090/diff/28001/chrome/browser/resources/ntp4/new_tab.css File chrome/browser/resources/ntp4/new_tab.css (right): https://chromiumcodereview.appspot.com/12038090/diff/28001/chrome/browser/resources/ntp4/new_tab.css#newcode439 chrome/browser/resources/ntp4/new_tab.css:439: -webkit-order: 2; I think the order should always remain ...
7 years, 10 months ago (2013-02-06 22:09:14 UTC) #25
dconnelly
https://chromiumcodereview.appspot.com/12038090/diff/28001/chrome/browser/resources/ntp4/new_tab.css File chrome/browser/resources/ntp4/new_tab.css (right): https://chromiumcodereview.appspot.com/12038090/diff/28001/chrome/browser/resources/ntp4/new_tab.css#newcode439 chrome/browser/resources/ntp4/new_tab.css:439: -webkit-order: 2; On 2013/02/06 22:09:14, Evan Stade wrote: > ...
7 years, 10 months ago (2013-02-06 23:09:24 UTC) #26
Evan Stade
https://chromiumcodereview.appspot.com/12038090/diff/28001/chrome/browser/resources/ntp4/new_tab.css File chrome/browser/resources/ntp4/new_tab.css (right): https://chromiumcodereview.appspot.com/12038090/diff/28001/chrome/browser/resources/ntp4/new_tab.css#newcode439 chrome/browser/resources/ntp4/new_tab.css:439: -webkit-order: 2; On 2013/02/06 23:09:24, dconnelly wrote: > On ...
7 years, 10 months ago (2013-02-06 23:15:43 UTC) #27
dconnelly1
It only moves to the left of the container (order: -1), just like the other ...
7 years, 10 months ago (2013-02-07 08:08:30 UTC) #28
dconnelly
On 2013/02/07 08:08:30, dconnelly_google.com wrote: > It only moves to the left of the container ...
7 years, 10 months ago (2013-02-11 09:38:48 UTC) #29
Evan Stade
sorry for delay. lgtm
7 years, 10 months ago (2013-02-12 01:38:26 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dconnelly@chromium.org/12038090/20008
7 years, 10 months ago (2013-02-12 10:50:27 UTC) #31
commit-bot: I haz the power
Retried try job too often on mac_rel for step(s) interactive_ui_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&number=98646
7 years, 10 months ago (2013-02-12 11:50:13 UTC) #32
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dconnelly@chromium.org/12038090/20008
7 years, 10 months ago (2013-02-12 12:58:32 UTC) #33
commit-bot: I haz the power
Retried try job too often on mac_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&number=98697
7 years, 10 months ago (2013-02-12 13:56:12 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dconnelly@chromium.org/12038090/20008
7 years, 10 months ago (2013-02-13 08:45:26 UTC) #35
commit-bot: I haz the power
7 years, 10 months ago (2013-02-13 11:41:56 UTC) #36
Message was sent while issue was closed.
Change committed as 182190

Powered by Google App Engine
This is Rietveld 408576698