Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem wwith md-close-on-click when md-fixed is true #318

Closed
djensen47 opened this issue Nov 20, 2016 · 5 comments
Closed

Problem wwith md-close-on-click when md-fixed is true #318

djensen47 opened this issue Nov 20, 2016 · 5 comments

Comments

@djensen47
Copy link

  1. Create a side nav that is fixed and close on click
<md-sidenav view-model.ref="sideNav" md-fixed="true" md-close-on-click="true">
  ...
</md-sidenav>
  1. Resize browser to large so that the fixed side nav is visible
  2. Click an item in the list and nav does not close (good)
  3. Resize browser to medium or smaller so that the sidenav hides
  4. Open sidenav
  5. Click an item in the list

Expected result: the sidenav closes

Actual result: the sidenav remains open

@djensen47
Copy link
Author

djensen47 commented Nov 20, 2016

I think this line needs to be changed from:

 closeOnClick: (this.ref.mdFixed ? false : getBooleanFromAttributeValue(this.ref.mdCloseOnClick)),

to

 closeOnClick: (this.ref.mdFixed && window.innerWidth > 992 ? false : getBooleanFromAttributeValue(this.ref.mdCloseOnClick)),

I'm not a fan of hardcoding values like 992 but it turns out that the Materialize library does it all over the place. When in Rome? ¯_(ツ)_/¯

@Thanood
Copy link
Collaborator

Thanood commented Nov 20, 2016

I think you're right. That's one of the reasons the component is still marked as "work in progress" and I completely forgot about that, sorry. The current closeOnClick behaviour is more of a cheap workaround to an issue where closeOnClick would initially even close an open fixed side-nav.

I've had some more ideas with side-nav, like a service monitoring certain breakpoints (like 992px) and optionally adjusting elements via a custom attribute when these values change - for example body padding. But your idea is a nice and easy start. 😃

I guess we could even make the breakpoint an option so we don't rely on hardcoded values at all (I know Materialize itself still does that)..

@djensen47
Copy link
Author

👌 No problem. Any chance we can get a temporary workaround, like above, before the final is done?

@Thanood
Copy link
Collaborator

Thanood commented Nov 20, 2016

Just upgrade, I've put it in 0.19.1 😃
Somehow, my sidenav overlays are above the sidenav. Not sure if it's only my app or if it's a bug.

I'll create a separate issue for that.

@djensen47
Copy link
Author

👍 Awesome! Thanks for the quick turnaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants