Code review request

[GoogleCodeExporter]

Branch name:
navbar-small-screens

Link to the relevant commit(s):
https://code.google.com/p/oppia/source/detail?r=785476519ef186197a23b3f611677aae
e46cacef&name=navbar-small-screens

Purpose of code changes on this branch:
Remove the breadcrumb on small screens, and keep the user icon on the right.

When reviewing my code changes, please focus on:
Everything.
The user icon dropdown now looks a bit strange on small devices with the extra 
whitespace on the left, this is because of the various style changes bootstrap 
adds to the navbar-nav class on small screens. Removing this class would cause 
the dropdown to be incompatible with many custom styles. I think it is okay to 
leave this as it is, and make further changes when we do the mobile view. What 
do you think?

After the review, I'll merge this branch into: develop

Original issue reported on code.google.com by wxy.xi...@gmail.com on 25 Feb 2015 at 2:25

[GoogleCodeExporter]

Amit could you please look at this? It fixes the navbar on small screens.

Thanks!

Original comment by s...@google.com on 25 Feb 2015 at 8:04

[GoogleCodeExporter]

Hey Sean,

There are a few things that seem broken here:

1) The Tagline (Your personal tutor for...) overlaps with the title
2) The user's icon has that huge gap on the left when you open the dropdown, as 
you pointed out.
3) The items in the dropdown menu float to the right, leaving a giant blank 
white space in the left part of the dropdown menu.

I think these things would need to be fixed before it could be merged. A few 
more thoughts:

-For the reader view, it looks like there is enough space for the feedback and 
share icons. Is it possible to include them in the navbar as well?
-On small screens, it would be nice if the attribution at the bottom of 
explorations weren't there, since it gets in the way of the experience. Do you 
think it's alright to remove it?

-In the splash page, would it be possible to turn the search bar into a search 
icon on small screens, and then replace the navbar with a search box when the 
icon is pressed? I'm sure this one isn't trivial so feel free to put it off for 
now, but I think it would be good to have this eventually on mobile.
-In the gallery, the descriptions should be hidden for explorations for the 
time being, so that they don't spill out of the exploration cards.

Thanks!

Original comment by amitdeut...@google.com on 25 Feb 2015 at 10:10

[GoogleCodeExporter]

Update: Amit, I just spoke to Xinyu, and it looks like adding the 
feedback/share icons in the way you describe is hard.

I see two options:
(a) remove them altogether
(b) show them in a dropdown (this is also the case for the editor view).

Which do you prefer?

Original comment by s...@seanlip.org on 26 Feb 2015 at 2:53

[GoogleCodeExporter]

How would the drop down be accessed, would we need a new icon?

Also, it's worth testing sharing and feedback on mobile first, because if those 
are broken on small screens anyways then it's probably not worth the effort to 
implement a new drop down.

Original comment by amitdeut...@google.com on 26 Feb 2015 at 3:05

[GoogleCodeExporter]

Yes, another dropdown icon will be needed.

Original comment by wxy.xi...@gmail.com on 26 Feb 2015 at 3:18

[GoogleCodeExporter]

Hi, I've made changes here: 
https://code.google.com/p/oppia/source/detail?r=ca7531a7b8050134beb70014582da2fc
dc44b0c6&name=navbar-small-screens, please take a look.

Original comment by wxy.xi...@gmail.com on 1 Mar 2015 at 6:03

[GoogleCodeExporter]

Thanks, Xinyu! I think this definitely looks better, and the code looks good. I 
noticed only one thing: when the user is logged out, the "Sign in" dropdown 
label at the top right looks weird. I've attached a picture to illustrate. Does 
this happen for you too?

Amit, I think you should be the final approver for this, so if you could take a 
look, that would be great! (One note: if it looks good as-is but there are 
further separate enhancements needed, let's do those further ones in a separate 
commit/branch, and merge this one in.)

Original comment by s...@seanlip.org on 1 Mar 2015 at 8:33

Attachments:

• issue-627.png

[GoogleCodeExporter]

Oops, I forgot to add the class to the 'Sign in' text. Please see the next 
commit in the same branch.

Original comment by wxy.xi...@gmail.com on 1 Mar 2015 at 9:29

[GoogleCodeExporter]

Thanks, looks good!

I tested this more and it's looking good, so in the interests of reducing skew 
I'm going to take the liberty of just merging this into develop. Amit, if you 
have other comments, please feel free to make them here and they can be 
addressed separately -- thanks!

Original comment by s...@seanlip.org on 1 Mar 2015 at 11:32

[GoogleCodeExporter]

Just checked it out, it works like a charm. Thank you so much!

Original comment by amitdeut...@google.com on 2 Mar 2015 at 4:05

[GoogleCodeExporter]

Original comment by amitdeut...@google.com on 3 Mar 2015 at 9:19

• Changed state: Done