toggle-topbar not working #1

Closed
leonistor opened this Issue Jul 1, 2013 · 7 comments

Projects

None yet

4 participants

@leonistor

I'm using meteorite 0.6.6 on Linux Ubuntu 13, and I'm trying to implement a simple layout using the Workspace Template. The layout and the responsive behaviour seems to be working fine, no errors in the console, except when resizing the browser (Chrome 28 or Firefox 22) to a smaller width (less than 955px), and the menu in the top bar is "transformed" into the "MENU button", this button is inactive: clicking on it will not expand the menu.
The funny thing is that if I type Foundation.init() in the JavaScript console, after the page loads, the menu button will start to behave correctly! Also, if I take the same HTML and place it in a static page, the menu button will function correctly.

@nsmeta
Member
nsmeta commented Jul 1, 2013

Thank you for logging this issue. I've been able to reproduce it.
I have an idea for a fix, and will get back to you when it's fixed (shouldn't take long, possibly tomorrow).

Out of curiosity, is the carousel working fine for you?

@nsmeta nsmeta added a commit that referenced this issue Jul 1, 2013
@nsmeta nsmeta Issue #1 fixed cc088f8
@nsmeta
Member
nsmeta commented Jul 1, 2013

This should be fixed now. Try running mrt update to pull the latest changes. Thank you for reporting this issue!

@nsmeta nsmeta closed this Jul 1, 2013
@leonistor

The hack is ugly like a troll, but it's effective: the menu works as expected on all screen sizes, and so does the image carousel, Orbit. Thank you as well!

@mem0master

what do you think about :

remove the init-foundation.html & .js form packages/zurb-foudation/html && /js

update the packages.js in packages/zurb-foudation/ to remove init-foundation.js

basicly remove all the init stuff and let the user do the initiation in his client.js like so :

Meteor.startup(function () {
$(document).foundation();
//other stuff to do once the app is ready
});

it work like a charm and there's no overhead (that i can see), the way thing are setup right now aren't good enough : if we wait for a the template to render so we can init the script chances are the Meteor app is already ready ! && correct me if i'm wrong Meteor.startup() run before rendering i think (although i'm not 100% sure)

  • we save some memory space by not storing the document in a var that is not accessible elsewhere and we're saving some computation cycles (js + jquery + template stuff in the init code) that may not be noticeable at early stages of an app life but when your stuck with 1000+ lines of code in the client part, code optimization become a necessity (for mobile apps at least)
@nsmeta
Member
nsmeta commented Jul 7, 2013

Basically, this boils down to convenience vs. performance.
The idea of this smart-package is to be as friendly as possible: just run mrt add zurb-foundation and you don't have to do any additional steps to make it work.

Now, I agree that there's no need for jQuery document.ready inside of the init template, as templates will be attached to the DOM when DOM is ready. I will fix this up for the next release.

if we wait for a the template to render so we can init the script chances are the Meteor app is already ready

I've placed a few console.log statements throughout the smart-package and a test meteor app that is using it to see the "lifecycle".

[smart-package] jQuery document.ready
[app] jQuery document.ready
[smart package] Meteor.startup
[app] Meteor.startup
[smart-package] template rendered
[app] myTemplate rendered
[app] myTemplate2 rendered

Yes. Meteor.startup in the app will be called before the smart-package template rendered callback. But the smart-package's template rendered callback will be called before any app-specific template is rendered. Since most (if not all) meteor apps will use templates, it is acceptable, in my opinion, to initialize foundation when the dummy template is rendered (before app-specific templates).

For future release, I will add a configuration option that will allow to disable this auto- initialization if a dev desires so to gain more flexibility.

Thank you for sharing your concerns, and please do let me know your thoughts on the proposed solution.

@lezed1
lezed1 commented Aug 12, 2014

This is now a problem again!

@nsmeta nsmeta reopened this Aug 12, 2014
@nsmeta
Member
nsmeta commented Aug 12, 2014

Upgraded to 5.3.3. Quick test did not reveal any problems.

Feel free to re-open this with further details if you're still experiencing the issue. Thanks!

@nsmeta nsmeta closed this Aug 12, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment