fix(nav-schematics): Subscribe to breakpoint observable #11310
Conversation
Otherwise, we are only checking if the observable is defined rather than actually checking the breakpoint.
This follow the Google guidelines for naming observables.
googlebot
added
the
cla: yes
| @@ -53,6 +53,6 @@ import { Observable } from 'rxjs'; | |||
| changeDetection: ChangeDetectionStrategy.<%= changeDetection %><% } %> | |||
| }) | |||
| export class <%= classify(name) %>Component { | |||
| isHandset: Observable<BreakpointState> = this.breakpointObserver.observe(Breakpoints.Handset); | |||
| isHandset$: Observable<BreakpointState> = this.breakpointObserver.observe(Breakpoints.Handset); | |||
There was a problem hiding this comment.
Since all the usages of isHandset are checking the matches, it would be cleaner if we added a .pipe(map(result => result.matches)) here. That way we can use isHandset$ | async in the templates, rather than (isHandset$ | async)!.matches.
|
@crisbeto Updated! |
crisbeto
added
pr: lgtm
action: merge
| @@ -53,6 +54,12 @@ import { Observable } from 'rxjs'; | |||
| changeDetection: ChangeDetectionStrategy.<%= changeDetection %><% } %> | |||
| }) | |||
| export class <%= classify(name) %>Component { | |||
| isHandset$: Observable<BreakpointState> = this.breakpointObserver.observe(Breakpoints.Handset); | |||
|
|
|||
| isHandset$: Observable<BreakpointState> = this.breakpointObserver.observe(Breakpoints.Handset) | |||
There was a problem hiding this comment.
This Observable has been changed to a boolean.
There was a problem hiding this comment.
There has already been a PR to fix it.
There was a problem hiding this comment.
Apologies, I updated to latest and the issue was still present. Found the PR which fixes it now. 👍
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
No description provided.