Skip to content
This repository has been archived by the owner. It is now read-only.

bugfix hamburger menu that is always open (#2195) #2198

Merged
merged 2 commits into from Jan 30, 2018

Conversation

Projects
None yet
3 participants
@jenweber
Copy link
Contributor

commented Jan 29, 2018

I added the collapse js to bootstrap-dropdown and called collapse in a script tag at the bottom of the _header.erb file. Opened here in case someone has a better plan.

Previous behavior (present in production) is that the hamburger menu always stays open in mobile 馃槦 I added some jquery and bootstrap to get it working like a normal hamburger menu - renders closed, click to open. bootstrap-dropdown is all just bootstrap basic boilerplate.

Test by running it locally and making the screen tiny. The dropdown flashes for a second but I am not sure how to fix it (and don't think we need to). Seems better than what we have now though.

Check to make sure that the navbar contents aren't still visible when closed:

screen shot 2018-01-29 at 10 39 48 pm

@jenweber jenweber changed the title WIP bugfix hamburger menu that is always open (#2195) bugfix hamburger menu that is always open (#2195) Jan 30, 2018

@locks

locks approved these changes Jan 30, 2018

@jessica-jordan
Copy link
Member

left a comment

This works well locally 馃憤 I left a few comments, but I'm unsure if the styles I commented on in this PR are actually relevant for the fix 馃 It might be worth to take another look on that in an upcoming PR

margin-left: 2px;
vertical-align: middle;
border-top: 4px dashed;
border-top: 4px solid \9;

This comment has been minimized.

Copy link
@jessica-jordan

jessica-jordan Jan 30, 2018

Member

The border-top property is duplicated and I assume the \9 could be replaced with a border-style or a border-color - what would work best here?

This comment has been minimized.

Copy link
@jenweber

jenweber Jan 30, 2018

Author Contributor

This is just bootstrap boilerplate so I'm inclined to leave it as-is unless it causes a problem

This comment has been minimized.

Copy link
@jessica-jordan

jessica-jordan Jan 30, 2018

Member

Yes, sounds good!

font-size: 14px;
text-align: left;
background-color: #ffffff;
border: 1px solid #cccccc;

This comment has been minimized.

Copy link
@jessica-jordan

jessica-jordan Jan 30, 2018

Member

Is there a variable already available for this color that could be reused?

list-style: none;
font-size: 14px;
text-align: left;
background-color: #ffffff;

This comment has been minimized.

Copy link
@jessica-jordan

jessica-jordan Jan 30, 2018

Member

Although pure white is straight-forward color, might it make sense to use a variable here (if available) to make sure that we can easily replace it in the future when the style guide changes?

@jenweber

This comment has been minimized.

Copy link
Contributor Author

commented Jan 30, 2018

@jessica-jordan I don't think we should refactor the bootstrap stylesheets themselves, as this is a stop gap measure until we have Ember-styleguide together. The CSS is just a copy-paste from the bootstap website

@jessica-jordan jessica-jordan merged commit a9987b8 into emberjs:master Jan 30, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can鈥檛 perform that action at this time.