code review 36500046: go-tour: tour redesign

go-tour: tour redesign

The web UI has been implemented from scratch using AngularJS and fetches the content of the modules via a REST APi.

The tour has been split into different modules composed by a list of lessons.

When a new lesson is added it will be accessible publicly, but it won't appear in the list of modules until the corresponding `lesson` element is added to list.html.

The new version is accessible at http://campoy.cat/gotour

[francesc, 2014-01-02 23:16:03]

Hello r@golang.org, adg@golang.org (cc: golang-dev@googlegroups.com),

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

[gobot, 2014-01-02 23:17:35]

Replacing golang-dev with golang-codereviews.

To the author of this CL: 

If you are using 'hg mail -r golang-dev' to mail the CL, use simply 'hg mail'
instead.

If you did not name golang-dev explicitly and it was still added to the CL,
it means your working copy of the repo has a stale codereview.cfg
(or lib/codereview/codereview.cfg).
Please run 'hg sync' to update your client to the most recent codereview.cfg.
If the most recent codereview.cfg has defaultcc set to golang-dev instead of
golang-codereviews, please send a CL correcting it.

Thanks very much.

[kisielk, 2014-01-03 01:57:00]

I think I remember mentioning this at some point before, but at the end of the
intro the "modules" link on the main page just blinks the "Modules" text in the
upper right hand corner. Could it not simply take the reader to the modules
page?

Also for me on Chrome 31.0.1650.63 the back button does not work. Though it
doesn't work on the old tour either, it would be good if it did.

[francesc, 2014-01-03 02:07:07]

Thanks for the feedback!

I plan to fix the navigation issues (back not working) soon.

My TODO list: https://gist.github.com/campoy/8231046

And I'm experimenting with other options for the modules link, such as
highlighting and clicking automatically at the end:

Demo: http://test.campoy-test.appspot.com/#/module/welcome/4

Cheers,


On Thu, Jan 2, 2014 at 5:57 PM, <kamil.kisiel@gmail.com> wrote:

> I think I remember mentioning this at some point before, but at the end
> of the intro the "modules" link on the main page just blinks the
> "Modules" text in the upper right hand corner. Could it not simply take
> the reader to the modules page?
>
> Also for me on Chrome 31.0.1650.63 the back button does not work. Though
> it doesn't work on the old tour either, it would be good if it did.
>
> https://codereview.appspot.com/36500046/
>



-- 
--
Francesc Campoy
http://campoy.cat

[egon, 2014-01-03 10:14:06]

On 2014/01/02 23:16:03, gocampoy wrote:
> Hello mailto:r@golang.org, mailto:adg@golang.org (cc:
mailto:golang-dev@googlegroups.com),
> 
> I'd like you to review this change to
> https://code.google.com/p/go-tour

The bubble in terminal output wastes screen-space without any additional
benefit. Something with ".output .info { border-radius:0; margin: 0; } .output {
padding: 0; }" looks cleaner and more defined... alternatively that could be
fully expanded with fixed position gopher in the background.

Header looks rather empty; maybe put the current module up there... Then it may
be also possible to line-up the file tabs with the right-side content.

https://dl.dropboxusercontent.com/u/4300994/go/gotour-modifications.png

For navigation use some combination that doesn't require switching from the code
editor, e.g. Ctrl+Alt+Arrows. I think they aren't useful if you need to use
mouse/tab anyway.

Also, it works poorly on a tablet.

+ egon

[francesc, 2014-01-03 22:47:50]

Thanks for the feedback.

On Fri, Jan 3, 2014 at 2:14 AM, <egonelbre@gmail.com> wrote:

> On 2014/01/02 23:16:03, gocampoy wrote:
>
>> Hello mailto:r@golang.org, mailto:adg@golang.org (cc:
>>
> mailto:golang-dev@googlegroups.com),
>
>
>  I'd like you to review this change to
>> https://code.google.com/p/go-tour
>>
>
> The bubble in terminal output wastes screen-space without any additional
> benefit. Something with ".output .info { border-radius:0; margin: 0; }
> .output { padding: 0; }" looks cleaner and more defined... alternatively
> that could be fully expanded with fixed position gopher in the
> background.
>
> Good idea, I removed the padding, margin, etc to better use the space.


> Header looks rather empty; maybe put the current module up there... Then
> it may be also possible to line-up the file tabs with the right-side
> content.
>
> https://dl.dropboxusercontent.com/u/4300994/go/gotour-modifications.png

I plan on adding other features (login, etc) so I prefer to safe that space
for later on.

>
>
> For navigation use some combination that doesn't require switching from
> the code editor, e.g. Ctrl+Alt+Arrows. I think they aren't useful if you
> need to use mouse/tab anyway.
>
Good idea, I'm working on that.

>
> Also, it works poorly on a tablet.
>
While making it usable on mobile devices is not a top priority (only 5% of
traffic is mobile) I'm always interested in ways to improve it. I've
personally used it on my Nexus 7 and I was happy with the result. What do
you mean exactly by poorly?


>
> + egon
>
> https://codereview.appspot.com/36500046/
>



-- 
--
Francesc Campoy
http://campoy.cat

[egon, 2014-01-03 23:13:44]

On 2014/01/03 22:47:50, gocampoy wrote:
> Thanks for the feedback.
> 
> On Fri, Jan 3, 2014 at 2:14 AM, <mailto:egonelbre@gmail.com> wrote:
> 
> > On 2014/01/02 23:16:03, gocampoy wrote:
> >
> >> Hello mailto:r@golang.org, mailto:adg@golang.org (cc:
> >>
> > mailto:golang-dev@googlegroups.com),
> >
> >
> >  I'd like you to review this change to
> >> https://code.google.com/p/go-tour
> >>
> >
> > The bubble in terminal output wastes screen-space without any additional
> > benefit. Something with ".output .info { border-radius:0; margin: 0; }
> > .output { padding: 0; }" looks cleaner and more defined... alternatively
> > that could be fully expanded with fixed position gopher in the
> > background.
> >
> > Good idea, I removed the padding, margin, etc to better use the space.
> 
> 
> > Header looks rather empty; maybe put the current module up there... Then
> > it may be also possible to line-up the file tabs with the right-side
> > content.
> >
> > https://dl.dropboxusercontent.com/u/4300994/go/gotour-modifications.png
> 
> I plan on adding other features (login, etc) so I prefer to safe that space
> for later on.
> 
> >
> >
> > For navigation use some combination that doesn't require switching from
> > the code editor, e.g. Ctrl+Alt+Arrows. I think they aren't useful if you
> > need to use mouse/tab anyway.
> >
> Good idea, I'm working on that.
> 
> >
> > Also, it works poorly on a tablet.
> >
> While making it usable on mobile devices is not a top priority (only 5% of
> traffic is mobile) I'm always interested in ways to improve it. I've
> personally used it on my Nexus 7 and I was happy with the result. What do
> you mean exactly by poorly?

I was using iPad + Chrome; and the issues in arbitrary order:

* Prev/Next buttons and Toc items are small.
* Scrolling the whole page behaves weirdly... only the headers move when
dragging.
* When focusing on the editor, sometimes you'll only see 5 lines at a time.
* Sizing the terminal and code editor do not work.
* Due to the fixed page, Chrome tabs are hard to make disappear, which means a
lot of wasted space.
* The headers should be probably moved to the side when in landscape mode,
making more room for the editor.
* For portrait mode look how current tour.golang does it; it works rather well.

> 
> 
> >
> > + egon
> >
> > https://codereview.appspot.com/36500046/
> >
> 
> 
> 
> -- 
> --
> Francesc Campoy
> http://campoy.cat

[francesc, 2014-01-03 23:25:45]

Thank for the detailed feedback, I really appreciate it.

As I said mobile is not a top priority, but I'll write these issues down
and try to fix them in a second pass.

Cheers,


On Fri, Jan 3, 2014 at 3:13 PM, <egonelbre@gmail.com> wrote:

> On 2014/01/03 22:47:50, gocampoy wrote:
>
>> Thanks for the feedback.
>>
>
>  On Fri, Jan 3, 2014 at 2:14 AM, <mailto:egonelbre@gmail.com> wrote:
>>
>
>  > On 2014/01/02 23:16:03, gocampoy wrote:
>> >
>> >> Hello mailto:r@golang.org, mailto:adg@golang.org (cc:
>> >>
>> > mailto:golang-dev@googlegroups.com),
>> >
>> >
>> >  I'd like you to review this change to
>> >> https://code.google.com/p/go-tour
>> >>
>> >
>> > The bubble in terminal output wastes screen-space without any
>>
> additional
>
>> > benefit. Something with ".output .info { border-radius:0; margin: 0;
>>
> }
>
>> > .output { padding: 0; }" looks cleaner and more defined...
>>
> alternatively
>
>> > that could be fully expanded with fixed position gopher in the
>> > background.
>> >
>> > Good idea, I removed the padding, margin, etc to better use the
>>
> space.
>
>
>  > Header looks rather empty; maybe put the current module up there...
>>
> Then
>
>> > it may be also possible to line-up the file tabs with the right-side
>> > content.
>> >
>> >
>>
> https://dl.dropboxusercontent.com/u/4300994/go/gotour-modifications.png
>
>  I plan on adding other features (login, etc) so I prefer to safe that
>>
> space
>
>> for later on.
>>
>
>  >
>> >
>> > For navigation use some combination that doesn't require switching
>>
> from
>
>> > the code editor, e.g. Ctrl+Alt+Arrows. I think they aren't useful if
>>
> you
>
>> > need to use mouse/tab anyway.
>> >
>> Good idea, I'm working on that.
>>
>
>  >
>> > Also, it works poorly on a tablet.
>> >
>> While making it usable on mobile devices is not a top priority (only
>>
> 5% of
>
>> traffic is mobile) I'm always interested in ways to improve it. I've
>> personally used it on my Nexus 7 and I was happy with the result. What
>>
> do
>
>> you mean exactly by poorly?
>>
>
> I was using iPad + Chrome; and the issues in arbitrary order:
>
> * Prev/Next buttons and Toc items are small.
> * Scrolling the whole page behaves weirdly... only the headers move when
> dragging.
> * When focusing on the editor, sometimes you'll only see 5 lines at a
> time.
> * Sizing the terminal and code editor do not work.
> * Due to the fixed page, Chrome tabs are hard to make disappear, which
> means a lot of wasted space.
> * The headers should be probably moved to the side when in landscape
> mode, making more room for the editor.
> * For portrait mode look how current tour.golang does it; it works
> rather well.
>
>
>
>
>  >
>> > + egon
>> >
>> > https://codereview.appspot.com/36500046/
>> >
>>
>
>
>
>  --
>> --
>> Francesc Campoy
>> http://campoy.cat
>>
>
>
> https://codereview.appspot.com/36500046/
>



-- 
--
Francesc Campoy
http://campoy.cat

[francesc, 2014-01-04 03:01:01]

I improved the design specially for mobile (responsive design)

I'll be working on the navigation issues next, but feel free to send more
feedback about the current version: http://campoy.cat/gotour

Cheers,


On Fri, Jan 3, 2014 at 3:25 PM, Francesc Campoy Flores
<campoy@golang.org>wrote:

> Thank for the detailed feedback, I really appreciate it.
>
> As I said mobile is not a top priority, but I'll write these issues down
> and try to fix them in a second pass.
>
> Cheers,
>
>
> On Fri, Jan 3, 2014 at 3:13 PM, <egonelbre@gmail.com> wrote:
>
>> On 2014/01/03 22:47:50, gocampoy wrote:
>>
>>> Thanks for the feedback.
>>>
>>
>>  On Fri, Jan 3, 2014 at 2:14 AM, <mailto:egonelbre@gmail.com> wrote:
>>>
>>
>>  > On 2014/01/02 23:16:03, gocampoy wrote:
>>> >
>>> >> Hello mailto:r@golang.org, mailto:adg@golang.org (cc:
>>> >>
>>> > mailto:golang-dev@googlegroups.com),
>>> >
>>> >
>>> >  I'd like you to review this change to
>>> >> https://code.google.com/p/go-tour
>>> >>
>>> >
>>> > The bubble in terminal output wastes screen-space without any
>>>
>> additional
>>
>>> > benefit. Something with ".output .info { border-radius:0; margin: 0;
>>>
>> }
>>
>>> > .output { padding: 0; }" looks cleaner and more defined...
>>>
>> alternatively
>>
>>> > that could be fully expanded with fixed position gopher in the
>>> > background.
>>> >
>>> > Good idea, I removed the padding, margin, etc to better use the
>>>
>> space.
>>
>>
>>  > Header looks rather empty; maybe put the current module up there...
>>>
>> Then
>>
>>> > it may be also possible to line-up the file tabs with the right-side
>>> > content.
>>> >
>>> >
>>>
>> https://dl.dropboxusercontent.com/u/4300994/go/gotour-modifications.png
>>
>>  I plan on adding other features (login, etc) so I prefer to safe that
>>>
>> space
>>
>>> for later on.
>>>
>>
>>  >
>>> >
>>> > For navigation use some combination that doesn't require switching
>>>
>> from
>>
>>> > the code editor, e.g. Ctrl+Alt+Arrows. I think they aren't useful if
>>>
>> you
>>
>>> > need to use mouse/tab anyway.
>>> >
>>> Good idea, I'm working on that.
>>>
>>
>>  >
>>> > Also, it works poorly on a tablet.
>>> >
>>> While making it usable on mobile devices is not a top priority (only
>>>
>> 5% of
>>
>>> traffic is mobile) I'm always interested in ways to improve it. I've
>>> personally used it on my Nexus 7 and I was happy with the result. What
>>>
>> do
>>
>>> you mean exactly by poorly?
>>>
>>
>> I was using iPad + Chrome; and the issues in arbitrary order:
>>
>> * Prev/Next buttons and Toc items are small.
>> * Scrolling the whole page behaves weirdly... only the headers move when
>> dragging.
>> * When focusing on the editor, sometimes you'll only see 5 lines at a
>> time.
>> * Sizing the terminal and code editor do not work.
>> * Due to the fixed page, Chrome tabs are hard to make disappear, which
>> means a lot of wasted space.
>> * The headers should be probably moved to the side when in landscape
>> mode, making more room for the editor.
>> * For portrait mode look how current tour.golang does it; it works
>> rather well.
>>
>>
>>
>>
>>  >
>>> > + egon
>>> >
>>> > https://codereview.appspot.com/36500046/
>>> >
>>>
>>
>>
>>
>>  --
>>> --
>>> Francesc Campoy
>>> http://campoy.cat
>>>
>>
>>
>> https://codereview.appspot.com/36500046/
>>
>
>
>
> --
> --
> Francesc Campoy
> http://campoy.cat
>



-- 
--
Francesc Campoy
http://campoy.cat

[kisielk, 2014-01-04 03:06:20]

One thing that also kind of jumped out at me:

After completing the initial module and going to the Modules list, the
first module at the top of the list is Welcome. Instinctively this is what
I'm drawn to and click first, which then loads the "Congratulations" page
again. Then clicking next goes back to the module list.

Kamil


On Fri, Jan 3, 2014 at 7:01 PM, Francesc Campoy Flores
<campoy@golang.org>wrote:

> I improved the design specially for mobile (responsive design)
>
> I'll be working on the navigation issues next, but feel free to send more
> feedback about the current version: http://campoy.cat/gotour
>
> Cheers,
>
>
> On Fri, Jan 3, 2014 at 3:25 PM, Francesc Campoy Flores
<campoy@golang.org>wrote:
>
>> Thank for the detailed feedback, I really appreciate it.
>>
>> As I said mobile is not a top priority, but I'll write these issues down
>> and try to fix them in a second pass.
>>
>> Cheers,
>>
>>
>> On Fri, Jan 3, 2014 at 3:13 PM, <egonelbre@gmail.com> wrote:
>>
>>> On 2014/01/03 22:47:50, gocampoy wrote:
>>>
>>>> Thanks for the feedback.
>>>>
>>>
>>>  On Fri, Jan 3, 2014 at 2:14 AM, <mailto:egonelbre@gmail.com> wrote:
>>>>
>>>
>>>  > On 2014/01/02 23:16:03, gocampoy wrote:
>>>> >
>>>> >> Hello mailto:r@golang.org, mailto:adg@golang.org (cc:
>>>> >>
>>>> > mailto:golang-dev@googlegroups.com),
>>>> >
>>>> >
>>>> >  I'd like you to review this change to
>>>> >> https://code.google.com/p/go-tour
>>>> >>
>>>> >
>>>> > The bubble in terminal output wastes screen-space without any
>>>>
>>> additional
>>>
>>>> > benefit. Something with ".output .info { border-radius:0; margin: 0;
>>>>
>>> }
>>>
>>>> > .output { padding: 0; }" looks cleaner and more defined...
>>>>
>>> alternatively
>>>
>>>> > that could be fully expanded with fixed position gopher in the
>>>> > background.
>>>> >
>>>> > Good idea, I removed the padding, margin, etc to better use the
>>>>
>>> space.
>>>
>>>
>>>  > Header looks rather empty; maybe put the current module up there...
>>>>
>>> Then
>>>
>>>> > it may be also possible to line-up the file tabs with the right-side
>>>> > content.
>>>> >
>>>> >
>>>>
>>> https://dl.dropboxusercontent.com/u/4300994/go/gotour-modifications.png
>>>
>>>  I plan on adding other features (login, etc) so I prefer to safe that
>>>>
>>> space
>>>
>>>> for later on.
>>>>
>>>
>>>  >
>>>> >
>>>> > For navigation use some combination that doesn't require switching
>>>>
>>> from
>>>
>>>> > the code editor, e.g. Ctrl+Alt+Arrows. I think they aren't useful if
>>>>
>>> you
>>>
>>>> > need to use mouse/tab anyway.
>>>> >
>>>> Good idea, I'm working on that.
>>>>
>>>
>>>  >
>>>> > Also, it works poorly on a tablet.
>>>> >
>>>> While making it usable on mobile devices is not a top priority (only
>>>>
>>> 5% of
>>>
>>>> traffic is mobile) I'm always interested in ways to improve it. I've
>>>> personally used it on my Nexus 7 and I was happy with the result. What
>>>>
>>> do
>>>
>>>> you mean exactly by poorly?
>>>>
>>>
>>> I was using iPad + Chrome; and the issues in arbitrary order:
>>>
>>> * Prev/Next buttons and Toc items are small.
>>> * Scrolling the whole page behaves weirdly... only the headers move when
>>> dragging.
>>> * When focusing on the editor, sometimes you'll only see 5 lines at a
>>> time.
>>> * Sizing the terminal and code editor do not work.
>>> * Due to the fixed page, Chrome tabs are hard to make disappear, which
>>> means a lot of wasted space.
>>> * The headers should be probably moved to the side when in landscape
>>> mode, making more room for the editor.
>>> * For portrait mode look how current tour.golang does it; it works
>>> rather well.
>>>
>>>
>>>
>>>
>>>  >
>>>> > + egon
>>>> >
>>>> > https://codereview.appspot.com/36500046/
>>>> >
>>>>
>>>
>>>
>>>
>>>  --
>>>> --
>>>> Francesc Campoy
>>>> http://campoy.cat
>>>>
>>>
>>>
>>> https://codereview.appspot.com/36500046/
>>>
>>
>>
>>
>> --
>> --
>> Francesc Campoy
>> http://campoy.cat
>>
>
>
>
> --
> --
> Francesc Campoy
> http://campoy.cat
>

[ioe, 2014-01-04 05:18:24]

The dragging area between code and output is a bit small and it is not indicated
visually that you can drag. Found it by accident :-)

Needed the dragging at http://campoy-test.appspot.com/#/module/concurrency/1

In the module list at http://campoy-test.appspot.com/#/list
basic concepts,
methods and interfaces,
and concurrency.

might be very useful as links (or even a TOC) to anchors for the individual
module sections.

PS: I love the new overall design and modularization a lot!

[francesc, 2014-01-04 08:05:47]

On Fri, Jan 3, 2014 at 9:18 PM, <nightlyone@googlemail.com> wrote:

> The dragging area between code and output is a bit small and it is not
> indicated visually that you can drag. Found it by accident :-)
>
> Needed the dragging at
> http://campoy-test.appspot.com/#/module/concurrency/1
>
> better now?

> In the module list at http://campoy-test.appspot.com/#/list
> basic concepts,
> methods and interfaces,
> and concurrency.
>
> done!

> might be very useful as links (or even a TOC) to anchors for the
> individual module sections.
>
> PS: I love the new overall design and modularization a lot!
>
> thanks!



-- 
--
Francesc Campoy
http://campoy.cat

[ioe, 2014-01-04 08:12:51]

Yes, much better.

PS: Sorry for the bad quoting, that's just Google Groups :-/

On Saturday, January 4, 2014 9:05:45 AM UTC+1, Francesc Campoy Flores wrote:
> On Fri, Jan 3, 2014 at 9:18 PM,  <night...@googlemail.com> wrote:
> 
> The dragging area between code and output is a bit small and it is not
indicated visually that you can drag. Found it by accident :-)
> 
> 
> 
> 
> Needed the dragging at http://campoy-test.appspot.com/#/module/concurrency/1
> 
> 
> 
> better now? 
> In the module list at http://campoy-test.appspot.com/#/list
> 
> basic concepts,
> 
> methods and interfaces,
> 
> and concurrency.
> 
> 
> 
> done! 
> might be very useful as links (or even a TOC) to anchors for the individual
module sections.
> 
> 
> 
> PS: I love the new overall design and modularization a lot!
> 
> 
> 
> thanks! 
> 
> 
> 
> -- 
> 
> --
> Francesc Campoy
> 
> http://campoy.cat

[egon, 2014-01-04 14:50:32]

On 2014/01/04 03:01:01, gocampoy wrote:
> I improved the design specially for mobile (responsive design)
> 

The portrait mode on the mobile is much better now.

The landscape on tablet
https://dl.dropboxusercontent.com/u/4300994/go/landscape%20mode.png is still no
that good. I was thinking how to do it, and probably the best way is to push the
page to scrolling behavior and then put the code and output side-by-side (e.g.
http://egonelbre.com/proto/gotour/tour.html see how it changes when making
height smaller)... Or alternatively to keep it simpler use the same layout as
the mobile version.

+ egon

> I'll be working on the navigation issues next, but feel free to send more
> feedback about the current version: http://campoy.cat/gotour
> 
> Cheers,
> 
> 
> On Fri, Jan 3, 2014 at 3:25 PM, Francesc Campoy Flores
> <campoy@golang.org>wrote:
> 
> > Thank for the detailed feedback, I really appreciate it.
> >
> > As I said mobile is not a top priority, but I'll write these issues down
> > and try to fix them in a second pass.
> >
> > Cheers,
> >
> >
> > On Fri, Jan 3, 2014 at 3:13 PM, <mailto:egonelbre@gmail.com> wrote:
> >
> >> On 2014/01/03 22:47:50, gocampoy wrote:
> >>
> >>> Thanks for the feedback.
> >>>
> >>
> >>  On Fri, Jan 3, 2014 at 2:14 AM, <mailto:egonelbre@gmail.com> wrote:
> >>>
> >>
> >>  > On 2014/01/02 23:16:03, gocampoy wrote:
> >>> >
> >>> >> Hello mailto:r@golang.org, mailto:adg@golang.org (cc:
> >>> >>
> >>> > mailto:golang-dev@googlegroups.com),
> >>> >
> >>> >
> >>> >  I'd like you to review this change to
> >>> >> https://code.google.com/p/go-tour
> >>> >>
> >>> >
> >>> > The bubble in terminal output wastes screen-space without any
> >>>
> >> additional
> >>
> >>> > benefit. Something with ".output .info { border-radius:0; margin: 0;
> >>>
> >> }
> >>
> >>> > .output { padding: 0; }" looks cleaner and more defined...
> >>>
> >> alternatively
> >>
> >>> > that could be fully expanded with fixed position gopher in the
> >>> > background.
> >>> >
> >>> > Good idea, I removed the padding, margin, etc to better use the
> >>>
> >> space.
> >>
> >>
> >>  > Header looks rather empty; maybe put the current module up there...
> >>>
> >> Then
> >>
> >>> > it may be also possible to line-up the file tabs with the right-side
> >>> > content.
> >>> >
> >>> >
> >>>
> >> https://dl.dropboxusercontent.com/u/4300994/go/gotour-modifications.png
> >>
> >>  I plan on adding other features (login, etc) so I prefer to safe that
> >>>
> >> space
> >>
> >>> for later on.
> >>>
> >>
> >>  >
> >>> >
> >>> > For navigation use some combination that doesn't require switching
> >>>
> >> from
> >>
> >>> > the code editor, e.g. Ctrl+Alt+Arrows. I think they aren't useful if
> >>>
> >> you
> >>
> >>> > need to use mouse/tab anyway.
> >>> >
> >>> Good idea, I'm working on that.
> >>>
> >>
> >>  >
> >>> > Also, it works poorly on a tablet.
> >>> >
> >>> While making it usable on mobile devices is not a top priority (only
> >>>
> >> 5% of
> >>
> >>> traffic is mobile) I'm always interested in ways to improve it. I've
> >>> personally used it on my Nexus 7 and I was happy with the result. What
> >>>
> >> do
> >>
> >>> you mean exactly by poorly?
> >>>
> >>
> >> I was using iPad + Chrome; and the issues in arbitrary order:
> >>
> >> * Prev/Next buttons and Toc items are small.
> >> * Scrolling the whole page behaves weirdly... only the headers move when
> >> dragging.
> >> * When focusing on the editor, sometimes you'll only see 5 lines at a
> >> time.
> >> * Sizing the terminal and code editor do not work.
> >> * Due to the fixed page, Chrome tabs are hard to make disappear, which
> >> means a lot of wasted space.
> >> * The headers should be probably moved to the side when in landscape
> >> mode, making more room for the editor.
> >> * For portrait mode look how current tour.golang does it; it works
> >> rather well.
> >>
> >>
> >>
> >>
> >>  >
> >>> > + egon
> >>> >
> >>> > https://codereview.appspot.com/36500046/
> >>> >
> >>>
> >>
> >>
> >>
> >>  --
> >>> --
> >>> Francesc Campoy
> >>> http://campoy.cat
> >>>
> >>
> >>
> >> https://codereview.appspot.com/36500046/
> >>
> >
> >
> >
> > --
> > --
> > Francesc Campoy
> > http://campoy.cat
> >
> 
> 
> 
> -- 
> --
> Francesc Campoy
> http://campoy.cat

[francesc, 2014-01-06 01:13:41]

Hey gophers, thanks for all the feedback so far!

I've heavily refactored the code and fixed most (all?) of the bugs in the
previous version.
Please have a look and let me know what you think about.

http://campoy.cat/gotour

As always all feedback is welcome, but try to keep it friendly :-)


On Sat, Jan 4, 2014 at 6:50 AM, <egonelbre@gmail.com> wrote:

> On 2014/01/04 03:01:01, gocampoy wrote:
>
>> I improved the design specially for mobile (responsive design)
>>
>
>
> The portrait mode on the mobile is much better now.
>
> The landscape on tablet
> https://dl.dropboxusercontent.com/u/4300994/go/landscape%20mode.png is
> still no that good. I was thinking how to do it, and probably the best
> way is to push the page to scrolling behavior and then put the code and
> output side-by-side (e.g. http://egonelbre.com/proto/gotour/tour.html
> see how it changes when making height smaller)... Or alternatively to
> keep it simpler use the same layout as the mobile version.
>
> + egon
>
>
>  I'll be working on the navigation issues next, but feel free to send
>>
> more
>
>> feedback about the current version: http://campoy.cat/gotour
>>
>
>  Cheers,
>>
>
>
>  On Fri, Jan 3, 2014 at 3:25 PM, Francesc Campoy Flores
>> <campoy@golang.org>wrote:
>>
>
>  > Thank for the detailed feedback, I really appreciate it.
>> >
>> > As I said mobile is not a top priority, but I'll write these issues
>>
> down
>
>> > and try to fix them in a second pass.
>> >
>> > Cheers,
>> >
>> >
>> > On Fri, Jan 3, 2014 at 3:13 PM, <mailto:egonelbre@gmail.com> wrote:
>> >
>> >> On 2014/01/03 22:47:50, gocampoy wrote:
>> >>
>> >>> Thanks for the feedback.
>> >>>
>> >>
>> >>  On Fri, Jan 3, 2014 at 2:14 AM, <mailto:egonelbre@gmail.com>
>>
> wrote:
>
>> >>>
>> >>
>> >>  > On 2014/01/02 23:16:03, gocampoy wrote:
>> >>> >
>> >>> >> Hello mailto:r@golang.org, mailto:adg@golang.org (cc:
>> >>> >>
>> >>> > mailto:golang-dev@googlegroups.com),
>> >>> >
>> >>> >
>> >>> >  I'd like you to review this change to
>> >>> >> https://code.google.com/p/go-tour
>> >>> >>
>> >>> >
>> >>> > The bubble in terminal output wastes screen-space without any
>> >>>
>> >> additional
>> >>
>> >>> > benefit. Something with ".output .info { border-radius:0;
>>
> margin: 0;
>
>> >>>
>> >> }
>> >>
>> >>> > .output { padding: 0; }" looks cleaner and more defined...
>> >>>
>> >> alternatively
>> >>
>> >>> > that could be fully expanded with fixed position gopher in the
>> >>> > background.
>> >>> >
>> >>> > Good idea, I removed the padding, margin, etc to better use the
>> >>>
>> >> space.
>> >>
>> >>
>> >>  > Header looks rather empty; maybe put the current module up
>>
> there...
>
>> >>>
>> >> Then
>> >>
>> >>> > it may be also possible to line-up the file tabs with the
>>
> right-side
>
>> >>> > content.
>> >>> >
>> >>> >
>> >>>
>> >>
>>
> https://dl.dropboxusercontent.com/u/4300994/go/gotour-modifications.png
>
>> >>
>> >>  I plan on adding other features (login, etc) so I prefer to safe
>>
> that
>
>> >>>
>> >> space
>> >>
>> >>> for later on.
>> >>>
>> >>
>> >>  >
>> >>> >
>> >>> > For navigation use some combination that doesn't require
>>
> switching
>
>> >>>
>> >> from
>> >>
>> >>> > the code editor, e.g. Ctrl+Alt+Arrows. I think they aren't
>>
> useful if
>
>> >>>
>> >> you
>> >>
>> >>> > need to use mouse/tab anyway.
>> >>> >
>> >>> Good idea, I'm working on that.
>> >>>
>> >>
>> >>  >
>> >>> > Also, it works poorly on a tablet.
>> >>> >
>> >>> While making it usable on mobile devices is not a top priority
>>
> (only
>
>> >>>
>> >> 5% of
>> >>
>> >>> traffic is mobile) I'm always interested in ways to improve it.
>>
> I've
>
>> >>> personally used it on my Nexus 7 and I was happy with the result.
>>
> What
>
>> >>>
>> >> do
>> >>
>> >>> you mean exactly by poorly?
>> >>>
>> >>
>> >> I was using iPad + Chrome; and the issues in arbitrary order:
>> >>
>> >> * Prev/Next buttons and Toc items are small.
>> >> * Scrolling the whole page behaves weirdly... only the headers move
>>
> when
>
>> >> dragging.
>> >> * When focusing on the editor, sometimes you'll only see 5 lines at
>>
> a
>
>> >> time.
>> >> * Sizing the terminal and code editor do not work.
>> >> * Due to the fixed page, Chrome tabs are hard to make disappear,
>>
> which
>
>> >> means a lot of wasted space.
>> >> * The headers should be probably moved to the side when in
>>
> landscape
>
>> >> mode, making more room for the editor.
>> >> * For portrait mode look how current tour.golang does it; it works
>> >> rather well.
>> >>
>> >>
>> >>
>> >>
>> >>  >
>> >>> > + egon
>> >>> >
>> >>> > https://codereview.appspot.com/36500046/
>> >>> >
>> >>>
>> >>
>> >>
>> >>
>> >>  --
>> >>> --
>> >>> Francesc Campoy
>> >>> http://campoy.cat
>> >>>
>> >>
>> >>
>> >> https://codereview.appspot.com/36500046/
>>
>> >>
>> >
>> >
>> >
>> > --
>> > --
>> > Francesc Campoy
>> > http://campoy.cat
>> >
>>
>
>
>
>  --
>> --
>> Francesc Campoy
>> http://campoy.cat
>>
>
>
> https://codereview.appspot.com/36500046/
>



-- 
--
Francesc Campoy
http://campoy.cat

[kortschak, 2014-01-06 01:44:24]

Looking nice. There is still the issue though of non-play screens only
using half the window.

On Sun, 2014-01-05 at 17:13 -0800, Francesc Campoy Flores wrote:
> Hey gophers, thanks for all the feedback so far!
> 
> I've heavily refactored the code and fixed most (all?) of the bugs in the
> previous version.
> Please have a look and let me know what you think about.
> 
> http://campoy.cat/gotour
> 
> As always all feedback is welcome, but try to keep it friendly :-)

[francesc, 2014-01-06 01:56:31]

It's not an issue, it's a feature.

On Sun, Jan 5, 2014 at 5:44 PM, Dan Kortschak <dan.kortschak@adelaide.edu.au
> wrote:

> Looking nice. There is still the issue though of non-play screens only
> using half the window.
>
> On Sun, 2014-01-05 at 17:13 -0800, Francesc Campoy Flores wrote:
> > Hey gophers, thanks for all the feedback so far!
> >
> > I've heavily refactored the code and fixed most (all?) of the bugs in the
> > previous version.
> > Please have a look and let me know what you think about.
> >
> > http://campoy.cat/gotour
> >
> > As always all feedback is welcome, but try to keep it friendly :-)
>
>
>


-- 
--
Francesc Campoy
http://campoy.cat

[kortschak, 2014-01-06 01:58:58]

On Sun, 2014-01-05 at 17:56 -0800, Francesc Campoy Flores wrote:
> It's not an issue, it's a feature.
> 
A fissure?

[kisielk, 2014-01-06 08:01:01]

I noticed on my iPad that the template directives are briefly displayed on the
page while it renders. It's been a few months since I've touched AngularJS but I
believe you can use ngCloak to fix this.

On Sunday, January 5, 2014 5:13:40 PM UTC-8, Francesc Campoy Flores wrote:
> Hey gophers, thanks for all the feedback so far!
> 
> 
> I've heavily refactored the code and fixed most (all?) of the bugs in the
previous version.
> Please have a look and let me know what you think about.
> 
> 
> 
> http://campoy.cat/gotour
> 
> 
> As always all feedback is welcome, but try to keep it friendly :-)
> 
> 
> 
> 
> On Sat, Jan 4, 2014 at 6:50 AM,  <egon...@gmail.com> wrote:
> 
> 
> On 2014/01/04 03:01:01, gocampoy wrote:
> 
> 
> I improved the design specially for mobile (responsive design)
> 
> 
> 
> 
> 
> 
> The portrait mode on the mobile is much better now.
> 
> 
> 
> The landscape on tablet
> 
> https://dl.dropboxusercontent.com/u/4300994/go/landscape%20mode.png is
> 
> still no that good. I was thinking how to do it, and probably the best
> 
> way is to push the page to scrolling behavior and then put the code and
> 
> output side-by-side (e.g. http://egonelbre.com/proto/gotour/tour.html
> 
> see how it changes when making height smaller)... Or alternatively to
> 
> keep it simpler use the same layout as the mobile version.
> 
> 
> 
> + egon
> 
> 
> 
> 
> 
> I'll be working on the navigation issues next, but feel free to send
> 
> 
> more
> 
> 
> feedback about the current version: http://campoy.cat/gotour
> 
> 
> 
> 
> 
> Cheers,
> 
> 
> 
> 
> 
> 
> 
> On Fri, Jan 3, 2014 at 3:25 PM, Francesc Campoy Flores
> 
> <cam...@golang.org>wrote:
> 
> 
> 
> 
> 
> > Thank for the detailed feedback, I really appreciate it.
> 
> >
> 
> > As I said mobile is not a top priority, but I'll write these issues
> 
> 
> down
> 
> 
> 
> > and try to fix them in a second pass.
> 
> >
> 
> > Cheers,
> 
> >
> 
> >
> 
> 
> 
> > On Fri, Jan 3, 2014 at 3:13 PM, <mailto:egon...@gmail.com> wrote:
> 
> >
> 
> >> On 2014/01/03 22:47:50, gocampoy wrote:
> 
> >>
> 
> >>> Thanks for the feedback.
> 
> >>>
> 
> >>
> 
> >>  On Fri, Jan 3, 2014 at 2:14 AM, <mailto:egon...@gmail.com>
> 
> 
> 
> 
> wrote:
> 
> 
> >>>
> 
> >>
> 
> >>  > On 2014/01/02 23:16:03, gocampoy wrote:
> 
> >>> >
> 
> >>> >> Hello mailto:r...@golang.org, mailto:a...@golang.org (cc:
> 
> >>> >>
> 
> >>> > mailto:golan...@googlegroups.com),
> 
> >>> >
> 
> >>> >
> 
> >>> >  I'd like you to review this change to
> 
> >>> >> https://code.google.com/p/go-tour
> 
> >>> >>
> 
> >>> >
> 
> >>> > The bubble in terminal output wastes screen-space without any
> 
> >>>
> 
> >> additional
> 
> >>
> 
> >>> > benefit. Something with ".output .info { border-radius:0;
> 
> 
> margin: 0;
> 
> 
> >>>
> 
> >> }
> 
> >>
> 
> >>> > .output { padding: 0; }" looks cleaner and more defined...
> 
> >>>
> 
> >> alternatively
> 
> >>
> 
> >>> > that could be fully expanded with fixed position gopher in the
> 
> >>> > background.
> 
> >>> >
> 
> >>> > Good idea, I removed the padding, margin, etc to better use the
> 
> >>>
> 
> >> space.
> 
> >>
> 
> >>
> 
> >>  > Header looks rather empty; maybe put the current module up
> 
> 
> there...
> 
> 
> >>>
> 
> >> Then
> 
> >>
> 
> >>> > it may be also possible to line-up the file tabs with the
> 
> 
> right-side
> 
> 
> >>> > content.
> 
> >>> >
> 
> >>> >
> 
> >>>
> 
> >>
> 
> 
> https://dl.dropboxusercontent.com/u/4300994/go/gotour-modifications.png
> 
> 
> >>
> 
> >>  I plan on adding other features (login, etc) so I prefer to safe
> 
> 
> that
> 
> 
> >>>
> 
> >> space
> 
> >>
> 
> >>> for later on.
> 
> >>>
> 
> >>
> 
> >>  >
> 
> >>> >
> 
> >>> > For navigation use some combination that doesn't require
> 
> 
> switching
> 
> 
> >>>
> 
> >> from
> 
> >>
> 
> >>> > the code editor, e.g. Ctrl+Alt+Arrows. I think they aren't
> 
> 
> useful if
> 
> 
> >>>
> 
> >> you
> 
> >>
> 
> >>> > need to use mouse/tab anyway.
> 
> >>> >
> 
> >>> Good idea, I'm working on that.
> 
> >>>
> 
> >>
> 
> >>  >
> 
> >>> > Also, it works poorly on a tablet.
> 
> >>> >
> 
> >>> While making it usable on mobile devices is not a top priority
> 
> 
> (only
> 
> 
> >>>
> 
> >> 5% of
> 
> >>
> 
> >>> traffic is mobile) I'm always interested in ways to improve it.
> 
> 
> I've
> 
> 
> >>> personally used it on my Nexus 7 and I was happy with the result.
> 
> 
> What
> 
> 
> >>>
> 
> >> do
> 
> >>
> 
> >>> you mean exactly by poorly?
> 
> >>>
> 
> >>
> 
> >> I was using iPad + Chrome; and the issues in arbitrary order:
> 
> >>
> 
> >> * Prev/Next buttons and Toc items are small.
> 
> >> * Scrolling the whole page behaves weirdly... only the headers move
> 
> 
> when
> 
> 
> >> dragging.
> 
> >> * When focusing on the editor, sometimes you'll only see 5 lines at
> 
> 
> a
> 
> 
> >> time.
> 
> >> * Sizing the terminal and code editor do not work.
> 
> >> * Due to the fixed page, Chrome tabs are hard to make disappear,
> 
> 
> which
> 
> 
> >> means a lot of wasted space.
> 
> >> * The headers should be probably moved to the side when in
> 
> 
> landscape
> 
> 
> 
> 
> >> mode, making more room for the editor.
> 
> >> * For portrait mode look how current tour.golang does it; it works
> 
> >> rather well.
> 
> >>
> 
> >>
> 
> >>
> 
> >>
> 
> >>  >
> 
> >>> > + egon
> 
> >>> >
> 
> >>> > https://codereview.appspot.com/36500046/
> 
> >>> >
> 
> >>>
> 
> >>
> 
> >>
> 
> >>
> 
> 
> >>  --
> 
> >>> --
> 
> >>> Francesc Campoy
> 
> >>> http://campoy.cat
> 
> >>>
> 
> >>
> 
> >>
> 
> >> https://codereview.appspot.com/36500046/
> 
> 
> >>
> 
> >
> 
> >
> 
> >
> 
> > --
> 
> > --
> 
> > Francesc Campoy
> 
> > http://campoy.cat
> 
> >
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> --
> 
> --
> 
> Francesc Campoy
> 
> http://campoy.cat
> 
> 
> 
> 
> 
> 
> https://codereview.appspot.com/36500046/
> 
> 
> 
> 
> 
> -- 
> 
> --
> Francesc Campoy
> 
> http://campoy.cat

[gobot, 2014-01-08 20:16:12]

R=golang-dev (assigned by r@golang.org)

[francesc, 2014-01-14 17:57:38]

Gentle ping.

This is awaiting review.


On Wed, Jan 8, 2014 at 12:16 PM, <gobot@golang.org> wrote:

> R=golang-dev (assigned by r@golang.org)
>
> https://codereview.appspot.com/36500046/
>



-- 
--
Francesc Campoy
http://campoy.cat

[bradfitz, 2014-01-14 21:27:24]

I made it through one line of the review.  Rietveld won't show a unified diff
and is taking multiple seconds per page load.

Who do you want to review this?  adg, I assume?

https://codereview.appspot.com/36500046/diff/230001/app.yaml
File app.yaml (right):

https://codereview.appspot.com/36500046/diff/230001/app.yaml#newcode1
app.yaml:1: application: campoy-test
I think this isn't meant to go in.

[francesc, 2014-01-14 22:33:28]

I'm happy with any gopher reviewing it. Andrew saw an initial review and
asked me to have it reviewed before his return next month.

I reuploaded the cl, is it working better for you now? It works fast and
shows the diffs to me


On Tue, Jan 14, 2014 at 1:27 PM, <bradfitz@golang.org> wrote:

> I made it through one line of the review.  Rietveld won't show a unified
> diff and is taking multiple seconds per page load.
>
> Who do you want to review this?  adg, I assume?
>
>
>
> https://codereview.appspot.com/36500046/diff/230001/app.yaml
> File app.yaml (right):
>
> https://codereview.appspot.com/36500046/diff/230001/app.yaml#newcode1
> app.yaml:1: application: campoy-test
> I think this isn't meant to go in.
>
> https://codereview.appspot.com/36500046/
>



-- 
--
Francesc Campoy
http://campoy.cat

[francesc, 2014-01-14 22:34:56]

https://codereview.appspot.com/36500046/diff/230001/app.yaml
File app.yaml (right):

https://codereview.appspot.com/36500046/diff/230001/app.yaml#newcode1
app.yaml:1: application: campoy-test
On 2014/01/14 21:27:26, bradfitz wrote:
> I think this isn't meant to go in.

Done.

[alsodave, 2014-01-18 04:45:40]

https://codereview.appspot.com/36500046/diff/240001/content/welcome.article
File content/welcome.article (right):

https://codereview.appspot.com/36500046/diff/240001/content/welcome.article#n...
content/welcome.article:39: When you're ready to move on, click the
[[javascript:highlightAndClick(".next-page")][right arrow]] above or type the
`PageDown` key.
Minor thing: the right arrow (at least on my screen) is below

[francesc, 2014-01-22 01:51:17]

https://codereview.appspot.com/36500046/diff/240001/content/welcome.article
File content/welcome.article (right):

https://codereview.appspot.com/36500046/diff/240001/content/welcome.article#n...
content/welcome.article:39: When you're ready to move on, click the
[[javascript:highlightAndClick(".next-page")][right arrow]] above or type the
`PageDown` key.
On 2014/01/18 04:45:41, davidday wrote:
> Minor thing: the right arrow (at least on my screen) is below

Done.

[francesc, 2014-02-05 14:17:43]

On 2014/01/22 01:51:17, gocampoy wrote:
> https://codereview.appspot.com/36500046/diff/240001/content/welcome.article
> File content/welcome.article (right):
> 
>
https://codereview.appspot.com/36500046/diff/240001/content/welcome.article#n...
> content/welcome.article:39: When you're ready to move on, click the
> [[javascript:highlightAndClick(".next-page")][right arrow]] above or type the
> `PageDown` key.
> On 2014/01/18 04:45:41, davidday wrote:
> > Minor thing: the right arrow (at least on my screen) is below
> 
> Done.

PTAL

[alsodave, 2014-02-06 02:34:42]

Looks good.

I agree with the previous comments that the end of the Welcome module feels a
bit abrupt, but it sounds like you already have some plans for addressing that. 

To me, having "/list" as the home page probably makes more sense than jumping
straight into the welcome module. You could store state client-side to remember
which modules a user has done so that you could highlight the welcome module for
new users.

Also, given that "A Tour of Go" and "Modules" both link to the same place, I'd
consider dropping the Modules link altogether.

[adg, 2014-02-13 23:42:07]

Reviewing http://campoy.cat/gotour

Bug: load http://campoy-test.appspot.com/basics/15 and click "next lesson".
It goes to a blank page.

Bug: the "modules" link on this page
http://campoy-test.appspot.com/welcome/4 doesn't work.

Inconsistency: The "congratulations" slides are different for the first and
subsequent modules. In particular, the later modules' pages don't include a
link back to the "modules" page.

Behavior: I think that clicking "next" from the last page of a module
should always take you to the next module. It is disorienting otherwise.
The new ToC makes this seem logical, too.

Look and feel: just remove this text and links from the modules page: "The
tour covers the most important features of the language, mainly: basic
concepts, methods and interfaces, and concurrency."

[kortschak, 2014-02-14 00:14:14]

This has been asked before, but the response didn't really explain why
the change has been made.

What has happened that has invalidated the design decision that made
non-play pages full width?

https://code.google.com/p/go-tour/source/detail?r=f49c09fb38a8

[adg, 2014-02-14 00:20:56]

On 14 February 2014 11:14, Dan Kortschak <dan.kortschak@adelaide.edu.au>wrote:

> This has been asked before, but the response didn't really explain why
> the change has been made.
>
> What has happened that has invalidated the design decision that made
> non-play pages full width?
>

The intent with this new version of the tour is that it should be possible
to have multiple code "tabs" open, and to "pin" a code tab such that it is
visible regardless of the slide you're looking at. That makes the "full
page" mode less appropriate.

It's not something that I had considered deeply before writing this post,
though.

Do you have a concrete use case for the full-width mode?

Andrew

[kortschak, 2014-02-14 00:47:07]

On Fri, 2014-02-14 at 11:20 +1100, Andrew Gerrand wrote:
> The intent with this new version of the tour is that it should be
> possible to have multiple code "tabs" open, and to "pin" a code tab
> such that it is visible regardless of the slide you're looking at.
> That makes the "full page" mode less appropriate.
> 
> It's not something that I had considered deeply before writing this
> post, though.
> 
> Do you have a concrete use case for the full-width mode?

Ahh. OK, thanks. That makes a lot of sense.

No, I don't have a case for full-width that trumps that.

I take it the tabs and pinning functionality is not yet implemented.

Dan

[francesc, 2014-02-14 00:50:27]

Not yet, it shouldn't be too hard but it's part of a next phase.


On Thu, Feb 13, 2014 at 4:46 PM, Dan Kortschak <
dan.kortschak@adelaide.edu.au> wrote:

> On Fri, 2014-02-14 at 11:20 +1100, Andrew Gerrand wrote:
> > The intent with this new version of the tour is that it should be
> > possible to have multiple code "tabs" open, and to "pin" a code tab
> > such that it is visible regardless of the slide you're looking at.
> > That makes the "full page" mode less appropriate.
> >
> > It's not something that I had considered deeply before writing this
> > post, though.
> >
> > Do you have a concrete use case for the full-width mode?
>
> Ahh. OK, thanks. That makes a lot of sense.
>
> No, I don't have a case for full-width that trumps that.
>
> I take it the tabs and pinning functionality is not yet implemented.
>
> Dan
>
>


-- 
--
Francesc Campoy
http://campoy.cat

[kortschak, 2014-02-14 00:58:05]

On Thu, 2014-02-13 at 16:50 -0800, Francesc Campoy Flores wrote:
> Not yet, it shouldn't be too hard but it's part of a next phase.
> 
Cool. Thanks.

[francesc, 2014-02-14 23:08:03]

On Thu, Feb 13, 2014 at 3:41 PM, Andrew Gerrand <adg@golang.org> wrote:

> Reviewing http://campoy.cat/gotour
>
> Bug: load http://campoy-test.appspot.com/basics/15 and click "next
> lesson". It goes to a blank page.
>

fixed

>
> Bug: the "modules" link on this page
> http://campoy-test.appspot.com/welcome/4 doesn't work.
>

fixed

>
> Inconsistency: The "congratulations" slides are different for the first
> and subsequent modules. In particular, the later modules' pages don't
> include a link back to the "modules" page.
>
> fixed

> Behavior: I think that clicking "next" from the last page of a module
> should always take you to the next module. It is disorienting otherwise.
> The new ToC makes this seem logical, too.
>
agreed, fixed


> Look and feel: just remove this text and links from the modules page: "The
> tour covers the most important features of the language, mainly: basic
> concepts, methods and interfaces, and concurrency."
>

I don't agree with this, I like having a short description of what the tour
is. Also the links allow a faster access to other modules.

This will be more useful when we start adding extra modules.

-- 
--
Francesc Campoy
http://campoy.cat

[adg, 2014-02-15 06:22:57]

On 15 February 2014 10:08, Francesc Campoy Flores <campoy@golang.org> wrote:

> I don't agree with this, I like having a short description of what the
> tour is. Also the links allow a faster access to other modules.


It seems that way to you because you wrote it, but I bounce off it every
time I look at it.

If we need a better navigation mechanism in the future, we can add one.

Andrew

[gobot, 2014-02-18 05:40:20]

R=adg@golang.org (assigned by adg@golang.org)

[francesc, 2014-02-19 16:03:31]

All fixed, CL is up to date, and last version is on http://campoy.cat/gotour

Now working on mobile


On Mon, Feb 17, 2014 at 9:40 PM, <gobot@golang.org> wrote:

> R=adg@golang.org (assigned by adg@golang.org)
>
> https://codereview.appspot.com/36500046/
>



-- 
--
Francesc Campoy
http://campoy.cat

[adg, 2014-02-21 00:51:00]

When you use "page down" to navigate through the pages, the keypress seems
to first be captured by the code editor window and then the surrounding
page. So the code editor scrollbar appears briefly and the text jumps up a
few pixels before the page turn. Please make sure the event not handled by
the code editor and only by the surrounding page.

[francesc, 2014-02-21 01:14:46]

Good catch! fixed and uploaded


On Thu, Feb 20, 2014 at 4:50 PM, Andrew Gerrand <adg@golang.org> wrote:

> When you use "page down" to navigate through the pages, the keypress seems
> to first be captured by the code editor window and then the surrounding
> page. So the code editor scrollbar appears briefly and the text jumps up a
> few pixels before the page turn. Please make sure the event not handled by
> the code editor and only by the surrounding page.
>
>


-- 
--
Francesc Campoy
http://campoy.cat

[adg, 2014-02-24 01:19:57]

Please make syntax highlighting off by default.

[adg, 2014-02-24 03:14:13]

https://codereview.appspot.com/36500046/diff/370001/static/js/controllers.js
File static/js/controllers.js (right):

https://codereview.appspot.com/36500046/diff/370001/static/js/controllers.js#...
static/js/controllers.js:55: /*function scrollDown() {
delete the commented-out code

https://codereview.appspot.com/36500046/diff/370001/static/js/controllers.js#...
static/js/controllers.js:68: // TODO: move animation to directive
what does this mean?

https://codereview.appspot.com/36500046/diff/370001/static/js/directives.js
File static/js/directives.js (right):

https://codereview.appspot.com/36500046/diff/370001/static/js/directives.js#n...
static/js/directives.js:14: elm.attr('tabindex', 0);
what's this about?

https://codereview.appspot.com/36500046/diff/370001/static/js/directives.js#n...
static/js/directives.js:145: template: '<img class="nav"
src="/static/img/burger.png" alt="menu">',
shouldn't this be in the actual html files?

https://codereview.appspot.com/36500046/diff/370001/static/js/directives.js#n...
static/js/directives.js:149: console.log(toc);
delete

https://codereview.appspot.com/36500046/diff/370001/static/js/directives.js#n...
static/js/directives.js:151: var showing = toc.css('display') == 'none';
s/showing/visible/

https://codereview.appspot.com/36500046/diff/370001/static/js/directives.js#n...
static/js/directives.js:161: console.log('fullscreen:', fullScreen,
$('.toc').width(), $(window).width());
delete

https://codereview.appspot.com/36500046/diff/370001/static/js/directives.js#n...
static/js/directives.js:163: if (showing) $('#editor-container').hide();
$('#editor-container').[visible?'hide':'show']();

https://codereview.appspot.com/36500046/diff/370001/static/js/services.js
File static/js/services.js (right):

https://codereview.appspot.com/36500046/diff/370001/static/js/services.js#new...
static/js/services.js:100: // load all the lessons concurrently and try to
resolve every time
what does resolve mean in this context? is it a term of art of angularjs?

https://codereview.appspot.com/36500046/diff/370001/static/js/services.js#new...
static/js/services.js:108: $http.get('/lesson/' + lesson).then(
can't we just request all of them at once? surely that's faster than several
round-trips, even if they are concurrent.

[francesc, 2014-02-24 21:53:03]

PTAL

https://codereview.appspot.com/36500046/diff/370001/static/js/controllers.js
File static/js/controllers.js (right):

https://codereview.appspot.com/36500046/diff/370001/static/js/controllers.js#...
static/js/controllers.js:55: /*function scrollDown() {
On 2014/02/24 03:14:14, adg wrote:
> delete the commented-out code

Done.

https://codereview.appspot.com/36500046/diff/370001/static/js/controllers.js#...
static/js/controllers.js:68: // TODO: move animation to directive
On 2014/02/24 03:14:14, adg wrote:
> what does this mean?

old comment, using jquery in a controller is bad practice.

https://codereview.appspot.com/36500046/diff/370001/static/js/directives.js
File static/js/directives.js (right):

https://codereview.appspot.com/36500046/diff/370001/static/js/directives.js#n...
static/js/directives.js:14: elm.attr('tabindex', 0);
On 2014/02/24 03:14:14, adg wrote:
> what's this about?

divs need to be "tabbable" to catch keyboard events, adding tabindex=0 is
needed.

https://codereview.appspot.com/36500046/diff/370001/static/js/directives.js#n...
static/js/directives.js:145: template: '<img class="nav"
src="/static/img/burger.png" alt="menu">',
On 2014/02/24 03:14:14, adg wrote:
> shouldn't this be in the actual html files?

Done.

https://codereview.appspot.com/36500046/diff/370001/static/js/directives.js#n...
static/js/directives.js:149: console.log(toc);
On 2014/02/24 03:14:14, adg wrote:
> delete

Done.

https://codereview.appspot.com/36500046/diff/370001/static/js/directives.js#n...
static/js/directives.js:151: var showing = toc.css('display') == 'none';
On 2014/02/24 03:14:14, adg wrote:
> s/showing/visible/

Done.

https://codereview.appspot.com/36500046/diff/370001/static/js/directives.js#n...
static/js/directives.js:161: console.log('fullscreen:', fullScreen,
$('.toc').width(), $(window).width());
On 2014/02/24 03:14:14, adg wrote:
> delete

Done.

https://codereview.appspot.com/36500046/diff/370001/static/js/directives.js#n...
static/js/directives.js:163: if (showing) $('#editor-container').hide();
On 2014/02/24 03:14:14, adg wrote:
> $('#editor-container').[visible?'hide':'show']();

nice!

https://codereview.appspot.com/36500046/diff/370001/static/js/services.js
File static/js/services.js (right):

https://codereview.appspot.com/36500046/diff/370001/static/js/services.js#new...
static/js/services.js:100: // load all the lessons concurrently and try to
resolve every time
On 2014/02/24 03:14:14, adg wrote:
> what does resolve mean in this context? is it a term of art of angularjs?

resolving a promise means to set a final value (therefore firing the execution
of any 'then' callbacks.
see line 129

https://codereview.appspot.com/36500046/diff/370001/static/js/services.js#new...
static/js/services.js:108: $http.get('/lesson/' + lesson).then(
On 2014/02/24 03:14:14, adg wrote:
> can't we just request all of them at once? surely that's faster than several
> round-trips, even if they are concurrent.

Done.

[adg, 2014-02-25 06:05:45]

https://codereview.appspot.com/36500046/diff/390001/gotour/local.go
File gotour/local.go (right):

https://codereview.appspot.com/36500046/diff/390001/gotour/local.go#newcode108
gotour/local.go:108: staticHandler = http.FileServer(http.Dir(root))
why not just
  http.Handle("/static/", http.FileServer(http.Dir(root))
  http.Handle("/content/", http.FileServer(http.Dir(root))
??

https://codereview.appspot.com/36500046/diff/390001/gotour/local.go#newcode128
gotour/local.go:128: if strings.HasPrefix(r.URL.Path, "/content/") {
why are you re-implementing http.ServeMux?

https://codereview.appspot.com/36500046/diff/390001/gotour/local.go#newcode143
gotour/local.go:143: lesson := strings.TrimPrefix(r.URL.Path, "/lesson/")
use http.StripPrefix when registering the handler, instead

[adg, 2014-02-25 22:00:13]

http://campoy-test.appspot.com/welcome/1 doesn't appear to be serving
/favicon.ico

[adg, 2014-02-25 22:14:44]

https://codereview.appspot.com/36500046/diff/390001/gotour/tour.go
File gotour/tour.go (right):

https://codereview.appspot.com/36500046/diff/390001/gotour/tour.go#newcode88
gotour/tour.go:88: name := f[:len(f)-len(".article")]
name := strings.TrimPrefix(f, ".article")

[ioe, 2014-02-25 22:29:56]

On Tuesday, February 25, 2014 11:14:46 PM UTC+1, Andrew Gerrand wrote:
> gotour/tour.go:88: name := f[:len(f)-len(".article")]
> name := strings.TrimPrefix(f, ".article")

You mean

name := strings.TrimSuffix(f, ".article")

?

[adg, 2014-02-25 22:44:29]

Yeah that one


On 26 February 2014 09:29, <nightlyone@googlemail.com> wrote:

> On Tuesday, February 25, 2014 11:14:46 PM UTC+1, Andrew Gerrand wrote:
> > gotour/tour.go:88: name := f[:len(f)-len(".article")]
> > name := strings.TrimPrefix(f, ".article")
>
> You mean
>
> name := strings.TrimSuffix(f, ".article")
>
> ?
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>

[francesc, 2014-02-25 23:13:01]

On 2014/02/25 22:44:29, adg wrote:
> Yeah that one
> 
> 
> On 26 February 2014 09:29, <mailto:nightlyone@googlemail.com> wrote:
> 
> > On Tuesday, February 25, 2014 11:14:46 PM UTC+1, Andrew Gerrand wrote:
> > > gotour/tour.go:88: name := f[:len(f)-len(".article")]
> > > name := strings.TrimPrefix(f, ".article")
> >
> > You mean
> >
> > name := strings.TrimSuffix(f, ".article")
> >
> > ?
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "golang-codereviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to mailto:golang-codereviews+unsubscribe@googlegroups.com.
> > For more options, visit https://groups.google.com/groups/opt_out.
> >

PTAL

[francesc, 2014-02-25 23:13:48]

The problem with favicon.ico was that the src attr was missing /static/ at
the beginning.

Now working on a more go-like feel and look.


On Tue, Feb 25, 2014 at 3:13 PM, <campoy@golang.org> wrote:

> On 2014/02/25 22:44:29, adg wrote:
>
>> Yeah that one
>>
>
>
>  On 26 February 2014 09:29, <mailto:nightlyone@googlemail.com> wrote:
>>
>
>  > On Tuesday, February 25, 2014 11:14:46 PM UTC+1, Andrew Gerrand
>>
> wrote:
>
>> > > gotour/tour.go:88: name := f[:len(f)-len(".article")]
>> > > name := strings.TrimPrefix(f, ".article")
>> >
>> > You mean
>> >
>> > name := strings.TrimSuffix(f, ".article")
>> >
>> > ?
>> >
>> > --
>> > You received this message because you are subscribed to the Google
>>
> Groups
>
>> > "golang-codereviews" group.
>> > To unsubscribe from this group and stop receiving emails from it,
>>
> send an
>
>> > email to mailto:golang-codereviews+unsubscribe@googlegroups.com.
>>
>> > For more options, visit https://groups.google.com/groups/opt_out.
>> >
>>
>
> PTAL
>
> https://codereview.appspot.com/36500046/
>



-- 
--
Francesc Campoy
http://campoy.cat

[adg, 2014-02-26 00:30:02]

Here's one that's going to bother you: why does the code box have a scroll
bar, even when the whole program fits in the box?

[francesc, 2014-02-26 01:36:45]

surprisingly painful to solve ... but fixed now (and the resulting css is
even simpler, without !important)


On Tue, Feb 25, 2014 at 4:29 PM, Andrew Gerrand <adg@golang.org> wrote:

> Here's one that's going to bother you: why does the code box have a scroll
> bar, even when the whole program fits in the box?
>



-- 
--
Francesc Campoy
http://campoy.cat

[francesc, 2014-02-26 02:19:36]

synced, uploaded, and campoy.cat/gotour updated


On Tue, Feb 25, 2014 at 5:36 PM, Francesc Campoy Flores
<campoy@golang.org>wrote:

> surprisingly painful to solve ... but fixed now (and the resulting css is
> even simpler, without !important)
>
>
> On Tue, Feb 25, 2014 at 4:29 PM, Andrew Gerrand <adg@golang.org> wrote:
>
>> Here's one that's going to bother you: why does the code box have a
>> scroll bar, even when the whole program fits in the box?
>>
>
>
>
> --
> --
> Francesc Campoy
> http://campoy.cat
>



-- 
--
Francesc Campoy
http://campoy.cat

[adg, 2014-02-26 02:26:04]

LGTM

[francesc, 2014-02-26 02:28:49]

*** Submitted as https://code.google.com/p/go-tour/source/detail?r=1ad78fbb7ab4
***

go-tour: tour redesign

The web UI has been implemented from scratch using AngularJS and fetches the
content of the modules via a REST APi.

The tour has been split into different modules composed by a list of lessons.

When a new lesson is added it will be accessible publicly, but it won't appear
in the list of modules until the corresponding `lesson` element is added to
list.html.

The new version is accessible at http://campoy.cat/gotour

LGTM=adg
R=r, adg, kamil.kisiel, egonelbre, nightlyone, dan.kortschak, gobot, bradfitz,
davidday
CC=golang-codereviews
https://codereview.appspot.com/36500046

https://codereview.appspot.com/36500046/diff/390001/gotour/local.go
File gotour/local.go (right):

https://codereview.appspot.com/36500046/diff/390001/gotour/local.go#newcode108
gotour/local.go:108: staticHandler = http.FileServer(http.Dir(root))
On 2014/02/25 06:05:47, adg wrote:
> why not just
>   http.Handle("/static/", http.FileServer(http.Dir(root))
>   http.Handle("/content/", http.FileServer(http.Dir(root))
> ??

done

https://codereview.appspot.com/36500046/diff/390001/gotour/local.go#newcode128
gotour/local.go:128: if strings.HasPrefix(r.URL.Path, "/content/") {
On 2014/02/25 06:05:47, adg wrote:
> why are you re-implementing http.ServeMux?

Done.

https://codereview.appspot.com/36500046/diff/390001/gotour/local.go#newcode143
gotour/local.go:143: lesson := strings.TrimPrefix(r.URL.Path, "/lesson/")
On 2014/02/25 06:05:47, adg wrote:
> use http.StripPrefix when registering the handler, instead

It would make my code more complicated, since I'm using HandleFunc and there's
not equivalent of http.StripPrefix for HandlerFuncs

https://codereview.appspot.com/36500046/diff/390001/gotour/tour.go
File gotour/tour.go (right):

https://codereview.appspot.com/36500046/diff/390001/gotour/tour.go#newcode88
gotour/tour.go:88: name := f[:len(f)-len(".article")]
On 2014/02/25 22:14:46, adg wrote:
> name := strings.TrimPrefix(f, ".article")

Done.