sidenav update #384
sidenav update #384
Conversation
| this._super(...arguments); | ||
| if (this.get('navContainer')) { | ||
| let lockedOpen = this.get('navContainer').get('sideBar').get('locked-open'); | ||
| let lockedOpen = this.get('navContainer').get('sideBar').get('lockedOpen'); |
There was a problem hiding this comment.
Would there be a use case for lockedOpen being dynamic (i.e. changing after component creation)? I don't have one, but it would be easy to move this to didReceiveAttrs().
There was a problem hiding this comment.
this is on the toggle component. The toggle component is reaching the sidenav container for the lockedOpen. lockedOpen is never passed in to sidenav-toggle, so I don't see how we can use didReceiveAttrs?
There was a problem hiding this comment.
Ah, right. Difficult to bind attributes from components that are parent/child. No matter as the use case is sketchy anyhow.
There was a problem hiding this comment.
still need a better way to do this with the new service. shouldn't be difficult.
…alue. added some guards on the lockedOpen changes
| {{#paper-sidenav-toggle as |toggle|}} | ||
| {{#paper-button onClick=(action "toggleMenu" target=toggle) iconButton=true}} | ||
| {{#paper-sidenav-toggle class="hide-gt-sm"}} | ||
| {{#paper-button onClick=null iconButton=true}} |
There was a problem hiding this comment.
Is this idiomatic? Maybe it should still be:
That way you can also use a link or something instead and it's explicit what the purpose of the toggle+button combo is?
There was a problem hiding this comment.
Yeah, maybe that is more flexible as well.
to let other people review this