Issue 5797076: code review 5797076: godoc: use elastic layout for most pages (Closed)
|
Created:
13 years ago by adg Modified:
13 years ago Reviewers:
CC:
golang-dev, bradfitz, gri Visibility:
Public. |
Descriptiongodoc: use elastic layout for most pages
We use the absence of a Title to detect the front page.
I can't find other pages without titles, so this seems reasonable.
Patch Set 1 #Patch Set 2 : diff -r 426b1101b166 https://code.google.com/p/go #Patch Set 3 : diff -r 35814deaa2c9 https://code.google.com/p/go #
MessagesTotal messages: 8
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go
Sign in to reply to this message.
LGTM tons better, but now a lot of pages are too left-flushed and white on big monitors. I'm sure I'll love it on my little laptops, though. it's been awhile since I fought CSS, but isn't there a way to have a border if there's space, else get rid of it? a bunch of google sites do that these days, but maybe that's with javascript? On Mon, Mar 12, 2012 at 3:33 PM, <adg@golang.org> wrote: > Reviewers: golang-dev_googlegroups.com, > > Message: > Hello golang-dev@googlegroups.com, > > I'd like you to review this change to > https://code.google.com/p/go > > > Description: > godoc: use elastic layout for most pages > > We use the absence of a Title to detect the front page. > I can't find other pages without titles, so this seems reasonable. > > Please review this at http://codereview.appspot.com/**5797076/<http://codereview.appspot.com/5797076/> > > Affected files: > M doc/style.css > M lib/godoc/godoc.html > > > Index: doc/style.css > ==============================**==============================**======= > --- a/doc/style.css > +++ b/doc/style.css > @@ -129,19 +129,25 @@ > > div#topbar { > background: #E0EBF5; > + height: 64px; > } > > body { > text-align: center; > } > div#page, > -div#topbar .container { > +div#topbar > .container { > clear: both; > text-align: left; > margin-left: auto; > margin-right: auto; > width: 900px; > } > +div#page.wide, > +div#topbar > .wide { > + width: auto; > + padding: 0 20px; > +} > div#plusone { > float: right; > } > @@ -182,6 +188,8 @@ > } > > div#menu { > + float: right; > + min-width: 590px; > padding: 10px 0; > text-align: right; > } > Index: lib/godoc/godoc.html > ==============================**==============================**======= > --- a/lib/godoc/godoc.html > +++ b/lib/godoc/godoc.html > @@ -15,10 +15,9 @@ > </head> > <body> > > -<div id="topbar"><div class="container"> > +<div id="topbar"><div class="container{{if .Title}} wide{{end}}"> > > <form method="GET" action="/search"> > -<div id="heading"><a href="/">The Go Programming Language</a></div> > <div id="menu"> > <a href="/doc/">Documents</a> > <a href="/ref/">References</a> > @@ -27,13 +26,12 @@ > <a href="/help/">Help</a> > <input type="text" id="search" name="q" class="inactive" value="Search"> > </div> > +<div id="heading"><a href="/">The Go Programming Language</a></div> > </form> > > </div></div> > > -<div id="page"> > - > -<div id="content"> > +<div id="page"{{if .Title}} class="wide"{{end}}> > > {{with .Title}} > <div id="plusone"><g:plusone size="small" annotation="none"></g:plusone> > **</div> > @@ -52,8 +50,6 @@ > > </div> > > -</div> > - > <div id="copyright"> > Build version {{html .Version}}. > Except as noted, this content is licensed under a > > >
Sign in to reply to this message.
On Mon, Mar 12, 2012 at 3:38 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote: > LGTM > > tons better, but now a lot of pages are too left-flushed and white on big agreed. Also, the main page, with very small browser window appears to not have any margin on the left anymore; even on small screens I think we can afford 10pixels or so. As is, it doesn't look very aesthetically pleasing. - gri > monitors. I'm sure I'll love it on my little laptops, though. > > it's been awhile since I fought CSS, but isn't there a way to have a border > if there's space, else get rid of it? a bunch of google sites do that these > days, but maybe that's with javascript? > > > On Mon, Mar 12, 2012 at 3:33 PM, <adg@golang.org> wrote: >> >> Reviewers: golang-dev_googlegroups.com, >> >> Message: >> Hello golang-dev@googlegroups.com, >> >> I'd like you to review this change to >> https://code.google.com/p/go >> >> >> Description: >> godoc: use elastic layout for most pages >> >> We use the absence of a Title to detect the front page. >> I can't find other pages without titles, so this seems reasonable. >> >> Please review this at http://codereview.appspot.com/5797076/ >> >> Affected files: >> M doc/style.css >> M lib/godoc/godoc.html >> >> >> Index: doc/style.css >> =================================================================== >> --- a/doc/style.css >> +++ b/doc/style.css >> @@ -129,19 +129,25 @@ >> >> div#topbar { >> background: #E0EBF5; >> + height: 64px; >> } >> >> body { >> text-align: center; >> } >> div#page, >> -div#topbar .container { >> +div#topbar > .container { >> clear: both; >> text-align: left; >> margin-left: auto; >> margin-right: auto; >> width: 900px; >> } >> +div#page.wide, >> +div#topbar > .wide { >> + width: auto; >> + padding: 0 20px; >> +} >> div#plusone { >> float: right; >> } >> @@ -182,6 +188,8 @@ >> } >> >> div#menu { >> + float: right; >> + min-width: 590px; >> padding: 10px 0; >> text-align: right; >> } >> Index: lib/godoc/godoc.html >> =================================================================== >> --- a/lib/godoc/godoc.html >> +++ b/lib/godoc/godoc.html >> @@ -15,10 +15,9 @@ >> </head> >> <body> >> >> -<div id="topbar"><div class="container"> >> +<div id="topbar"><div class="container{{if .Title}} wide{{end}}"> >> >> <form method="GET" action="/search"> >> -<div id="heading"><a href="/">The Go Programming Language</a></div> >> <div id="menu"> >> <a href="/doc/">Documents</a> >> <a href="/ref/">References</a> >> @@ -27,13 +26,12 @@ >> <a href="/help/">Help</a> >> <input type="text" id="search" name="q" class="inactive" value="Search"> >> </div> >> +<div id="heading"><a href="/">The Go Programming Language</a></div> >> </form> >> >> </div></div> >> >> -<div id="page"> >> - >> -<div id="content"> >> +<div id="page"{{if .Title}} class="wide"{{end}}> >> >> {{with .Title}} >> <div id="plusone"><g:plusone size="small" >> annotation="none"></g:plusone></div> >> @@ -52,8 +50,6 @@ >> >> </div> >> >> -</div> >> - >> <div id="copyright"> >> Build version {{html .Version}}. >> Except as noted, this content is licensed under a >> >> >
Sign in to reply to this message.
On Mon, Mar 12, 2012 at 3:40 PM, Robert Griesemer <gri@golang.org> wrote: > On Mon, Mar 12, 2012 at 3:38 PM, Brad Fitzpatrick <bradfitz@golang.org> > wrote: > > LGTM > > > > tons better, but now a lot of pages are too left-flushed and white on big > > agreed. Also, the main page, with very small browser window appears to > not have any margin on the left anymore; even on small screens I think > we can afford 10pixels or so. As is, it doesn't look very > aesthetically pleasing. > Yeah, I would also give up a couple pixels on my small monitors. Ultimate pony request is dynamic, though.
Sign in to reply to this message.
On 13 March 2012 09:38, Brad Fitzpatrick <bradfitz@golang.org> wrote: > it's been awhile since I fought CSS, but isn't there a way to have a border > if there's space, else get rid of it? a bunch of google sites do that these > days, but maybe that's with javascript? There may be, but this is a bottomless rabbit hole that I'm not keen to plunge into right now. If you're happy with it as-is, let's leave it 'til after Go 1. The main driver for this change is that the codewalks were unreadable before, so this is particular change is a go1-must. On 13 March 2012 09:40, Robert Griesemer <gri@golang.org> wrote: > agreed. Also, the main page, with very small browser window appears to > not have any margin on the left anymore; even on small screens I think > we can afford 10pixels or so. As is, it doesn't look very > aesthetically pleasing. Done. PTAL
Sign in to reply to this message.
LGTM for css and pretty enough for me. Robert? On Mon, Mar 12, 2012 at 3:50 PM, Andrew Gerrand <adg@golang.org> wrote: > On 13 March 2012 09:38, Brad Fitzpatrick <bradfitz@golang.org> wrote: > > it's been awhile since I fought CSS, but isn't there a way to have a > border > > if there's space, else get rid of it? a bunch of google sites do that > these > > days, but maybe that's with javascript? > > There may be, but this is a bottomless rabbit hole that I'm not keen > to plunge into right now. If you're happy with it as-is, let's leave > it 'til after Go 1. The main driver for this change is that the > codewalks were unreadable before, so this is particular change is a > go1-must. > > On 13 March 2012 09:40, Robert Griesemer <gri@golang.org> wrote: > > agreed. Also, the main page, with very small browser window appears to > > not have any margin on the left anymore; even on small screens I think > > we can afford 10pixels or so. As is, it doesn't look very > > aesthetically pleasing. > > Done. PTAL >
Sign in to reply to this message.
LGTM i guess On Mon, Mar 12, 2012 at 3:33 PM, <adg@golang.org> wrote: > Reviewers: golang-dev_googlegroups.com, > > Message: > Hello golang-dev@googlegroups.com, > > I'd like you to review this change to > https://code.google.com/p/go > > > Description: > godoc: use elastic layout for most pages > > We use the absence of a Title to detect the front page. > I can't find other pages without titles, so this seems reasonable. > > Please review this at http://codereview.appspot.com/5797076/ > > Affected files: > M doc/style.css > M lib/godoc/godoc.html > > > Index: doc/style.css > =================================================================== > --- a/doc/style.css > +++ b/doc/style.css > @@ -129,19 +129,25 @@ > > div#topbar { > background: #E0EBF5; > + height: 64px; > } > > body { > text-align: center; > } > div#page, > -div#topbar .container { > +div#topbar > .container { > clear: both; > text-align: left; > margin-left: auto; > margin-right: auto; > width: 900px; > } > +div#page.wide, > +div#topbar > .wide { > + width: auto; > + padding: 0 20px; > +} > div#plusone { > float: right; > } > @@ -182,6 +188,8 @@ > } > > div#menu { > + float: right; > + min-width: 590px; > padding: 10px 0; > text-align: right; > } > Index: lib/godoc/godoc.html > =================================================================== > --- a/lib/godoc/godoc.html > +++ b/lib/godoc/godoc.html > @@ -15,10 +15,9 @@ > </head> > <body> > > -<div id="topbar"><div class="container"> > +<div id="topbar"><div class="container{{if .Title}} wide{{end}}"> > > <form method="GET" action="/search"> > -<div id="heading"><a href="/">The Go Programming Language</a></div> > <div id="menu"> > <a href="/doc/">Documents</a> > <a href="/ref/">References</a> > @@ -27,13 +26,12 @@ > <a href="/help/">Help</a> > <input type="text" id="search" name="q" class="inactive" value="Search"> > </div> > +<div id="heading"><a href="/">The Go Programming Language</a></div> > </form> > > </div></div> > > -<div id="page"> > - > -<div id="content"> > +<div id="page"{{if .Title}} class="wide"{{end}}> > > {{with .Title}} > <div id="plusone"><g:plusone size="small" > annotation="none"></g:plusone></div> > @@ -52,8 +50,6 @@ > > </div> > > -</div> > - > <div id="copyright"> > Build version {{html .Version}}. > Except as noted, this content is licensed under a > >
Sign in to reply to this message.
*** Submitted as http://code.google.com/p/go/source/detail?r=bced5db94f81 *** godoc: use elastic layout for most pages We use the absence of a Title to detect the front page. I can't find other pages without titles, so this seems reasonable. R=golang-dev, bradfitz, gri CC=golang-dev http://codereview.appspot.com/5797076
Sign in to reply to this message.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
