-
Notifications
You must be signed in to change notification settings - Fork 5
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
Klarocca/side menu #144
Klarocca/side menu #144
Conversation
outOfViewport=this.outOfViewport | ||
rules=this.rules | ||
)}} | ||
{{#if @showMenuButton}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you put this conditional into the nypr-o-header/left.hbs
template instead, you won't need to use two separate components or duplicate this block
data-test-main-header | ||
as |header| | ||
> | ||
<header.left data-test-header-left as |left|> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the change suggested above this part of the example would become
<header.left @showMenuButton=false as |left|>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we make this change, we'd have to set the showMenuButton=true or =false for all the headers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can have a default, the same as if we set it in the top level.
Added an option to not have the side menu in the header like we discussed