search

quixoticduck / Roundhill

0 stars 0 forks source link

Navigation hiding #1

Open quixoticduck opened 9 years ago

quixoticduck commented 9 years ago

Style being added of display:none bu javascript. We think it's slideToggle. A second click on mobile nav means that the nav is gone if the window is enlarged to non-mobile size, there is no nav.

DanTheBlue commented 9 years ago

Hey,

Just thought I would go over the problem and how we fixed it, mainly as I think near the end we had to rush, and might mean it may not have made complete sense (if it did however, feel free to ignore me).

So, it seems that the jquery slideToggle() applied the inline style of display: none; which broke the whole menu when it was no longer the small version.

Initially, to fix this, we tried to remove the style attribute from the nav item, but this is where things went wrong :(

As is the nature of javascript, it is asynchronous (meaning essentially it doesnt wait for the previous function to finish before using the next).

Normally this is fine, but it ended up we were removing the style before it had been added, meaning it didnt work.

Andy figured this out I believe, and used a callback (essentially once the function was finished, it called the next function) which in turn then disabled the inline style again.

Anyway, hopefully your issue is now fixed. Feel free to ping some issues here and if I have a chance ill have a look, but glad we finally got to the end of this for you (even if it did take Andys help in the end).

Again, this was just in case it didnt make complete sense what we did, but feel free to ignore this otherwise!

Cheers,

Dan

quixoticduck commented 9 years ago

Hi, this is the final code that he did:

var isMenuDown = false; $('.button').on('click', function(){ if (isMenuDown) { $('nav').slideUp(function () { $('nav').removeAttr("style"); isMenuDown = false; }); } else { $('nav').slideDown(function () { isMenuDown = true; }); } //$('nav').removeAttr("style"); //console.log("slideToggle") });

So using slide up and down instead of toggle and making a variable for whether the mobile nav menu is expanded or not and checking and setting whether it's up or down, and also removing the attribute style after the menu has been opened? Which I think was causing the problem?

DanTheBlue commented 9 years ago

Yup, think that's exactly it . didn't realize he changed the function call as well, but good it works! Yeah that annoying attribute was the problem :( but these kind of errors happen all the time!

Glad it's all working now

On Sat, Mar 14, 2015, 10:52 PM quixoticduck notifications@github.com wrote:

Hi, this is the final code that he did:

var isMenuDown = false; $('.button').on('click', function(){ if (isMenuDown) { $('nav').slideUp(function () { $('nav').removeAttr("style"); isMenuDown = false; }); } else { $('nav').slideDown(function () { isMenuDown = true; }); } //$('nav').removeAttr("style"); //console.log("slideToggle") });

So using slide up and down instead of toggle and making a variable for whether the mobile nav menu is expanded or not and checking and setting whether it's up or down, and also removing the attribute style after the menu has been opened? Which I think was causing the problem?

— Reply to this email directly or view it on GitHub https://github.com/quixoticduck/Roundhill/issues/1#issuecomment-80751523 .