fix: side menu icon should not be visible in mobile when there is no menu #3665
Conversation
app/templates/public.hbs
Outdated
| </button> | ||
| {{#if isMenuOpen}} | ||
| {{public/side-menu class='toggle menu' event=model}} | ||
| {{#if (and (not-eq session.currentRouteName 'public.cfs.new-session') (not-eq session.currentRouteName 'public.cfs.new-speaker') (not-eq session.currentRouteName 'public.cfs.edit-speaker') (not-eq session.currentRouteName 'public.cfs.edit-session'))}} |
There was a problem hiding this comment.
Rather than using such conditionals in the templates, please use a computed property from controllers.
There was a problem hiding this comment.
The same condition is used in side-menu.hbs file for visibility of menu .
There was a problem hiding this comment.
I suspected so, @maze-runnar make a computed property and use it for the above condition too
There was a problem hiding this comment.
i have made the computed property and it's working fine . may i make changes here ?
|
@kushthedude please review |
app/controllers/public.js
Outdated
| @@ -11,6 +11,11 @@ export default Controller.extend({ | |||
| displayEndDate: computed('model.startsAtDate', 'model.endsAtDate', function() { | |||
| return !moment(this.model.startsAtDate).isSame(this.model.endsAtDate, 'minute'); | |||
| }), | |||
| displaySideMenu: computed('session.currentRouteName', function() { | |||
| if (this.get('session.currentRouteName')) { | |||
There was a problem hiding this comment.
should i migrate it to ES6?
There was a problem hiding this comment.
No, this.get('session') this.session
There was a problem hiding this comment.
in the same file :
smallLead: computed('session.currentRouteName', function() {
if (this.get('session.currentRouteName')) {
return this.get('session.currentRouteName') !== 'public.index';
}
}),
Should i change ES5 here too ?
|
@kushthedude should i open a seperate PR for migrating "app/controllers/public.js" to ES6. |
|
Complexity increasing per file
==============================
- app/controllers/public.js 3
See the complete overview on Codacy |
|
screenshot after above change . |
kushthedude
requested review from
iamareebjamal
and removed request for
iamareebjamal
fixes : #3656
visible for other pages
Checklist
developmentbranch.