code review 6631057: godoc: add dropdown playground to nav bar (issue 6631057)

[a...@golang.org]

Reviewers: gri, dsymonds,

Message:
Hello g...@golang.org, dsym...@golang.org (cc:
golan...@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go


Description:
godoc: add dropdown playground to nav bar

Please review this at http://codereview.appspot.com/6631057/

Affected files:
M doc/godocs.js
M doc/style.css
M lib/godoc/godoc.html
M lib/godoc/package.html
M src/cmd/godoc/godoc.go

[Andrew Gerrand]

Here's how it looks.

[Andrew Gerrand]

+skybrian

This also re-introduces the fixed nav bar on larger screens that was
previous rolled back. I solved the issue with the original version;
anchors links now lead to the right place.

[dsym...@golang.org]

LGTM

not sure about the UI, but the code looks fine.


https://codereview.appspot.com/6631057/diff/11001/doc/godocs.js
File doc/godocs.js (right):

https://codereview.appspot.com/6631057/diff/11001/doc/godocs.js#newcode143
doc/godocs.js:143: if (!setup) {
maybe invert this and return early, outdenting the rest?

https://codereview.appspot.com/6631057/diff/11001/lib/godoc/godoc.html
File lib/godoc/godoc.html (right):

https://codereview.appspot.com/6631057/diff/11001/lib/godoc/godoc.html#newcode33
lib/godoc/godoc.html:33: <a id="playgroundButton" href="#" title="Show
Go Playground">Play</a>
unlike the other menu links, this doesn't go to a standalone page. can
this not just go to play.golang.org?

https://codereview.appspot.com/6631057/diff/11001/lib/godoc/godoc.html#newcode51
lib/godoc/godoc.html:51: <div class="output">Hello, 世界<pre>
having the output pre-populated means that a casual visitor hitting
"Run" won't see a change. I'm not sure this is a gain.

https://codereview.appspot.com/6631057/

[a...@golang.org]

https://codereview.appspot.com/6631057/diff/11001/doc/godocs.js
File doc/godocs.js (right):

https://codereview.appspot.com/6631057/diff/11001/doc/godocs.js#newcode143
doc/godocs.js:143: if (!setup) {

On 2012/10/09 23:56:06, dsymonds wrote:
> maybe invert this and return early, outdenting the rest?

Done.

https://codereview.appspot.com/6631057/diff/11001/lib/godoc/godoc.html
File lib/godoc/godoc.html (right):

https://codereview.appspot.com/6631057/diff/11001/lib/godoc/godoc.html#newcode33
lib/godoc/godoc.html:33: <a id="playgroundButton" href="#" title="Show
Go Playground">Play</a>

On 2012/10/09 23:56:06, dsymonds wrote:
> unlike the other menu links, this doesn't go to a standalone page. can
this not
> just go to play.golang.org?

Done.

Although the button will only appear for people that have javascript
enabled, and if they do it won't function as a link anyway.

https://codereview.appspot.com/6631057/diff/11001/lib/godoc/godoc.html#newcode51
lib/godoc/godoc.html:51: <div class="output">Hello, 世界<pre>

On 2012/10/09 23:56:06, dsymonds wrote:
> having the output pre-populated means that a casual visitor hitting
"Run" won't
> see a change. I'm not sure this is a gain.

Done.

https://codereview.appspot.com/6631057/

[g...@golang.org]

LGTM for the .go changes.

Can't speak for js.

https://codereview.appspot.com/6631057/

[skyb...@google.com]

I didn't see how the anchor problem was fixed. Is that related to the
"wide" class?

https://codereview.appspot.com/6631057/

[a...@golang.org]

Brian, see below


https://codereview.appspot.com/6631057/diff/13001/doc/style.css
File doc/style.css (right):

https://codereview.appspot.com/6631057/diff/13001/doc/style.css#newcode534
doc/style.css:534: position: fixed;
The trick is this. Making the #page be a fixed box that scrolls
internally, with #page .container taking the role of what #page used to
be.

https://codereview.appspot.com/6631057/

[a...@golang.org]

*** Submitted as
http://code.google.com/p/go/source/detail?r=f9f27066cb90 ***

godoc: add dropdown playground to nav bar

R=gri, dsymonds, skybrian
CC=golang-dev
http://codereview.appspot.com/6631057


http://codereview.appspot.com/6631057/

[Daniel Morsing]

On my 1080p monitor, the page div doesn't fill up the entire width of
the screen.

Screenshot attached.

[Andrew Gerrand]

Thanks very much for the report. I've created
http://codereview.appspot.com/6643062