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

Sidenav issues in safari with Breakpoint Observer. #12479

Open
Trent-Matthias opened this issue Aug 2, 2018 · 6 comments
Open

Sidenav issues in safari with Breakpoint Observer. #12479

Trent-Matthias opened this issue Aug 2, 2018 · 6 comments
Labels
area: cdk/layout area: material/sidenav P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@Trent-Matthias
Copy link

Trent-Matthias commented Aug 2, 2018

Bug, feature request, or proposal:

Bug

What is the expected behavior?

For the sidenav to properly respond to the breakpoint observer. It should hide the toggle sidenav button in the toolbar when the breakpoint is false.

What is the current behavior?

Does not hide the toggle sidenav button in Safari only.

What are the steps to reproduce?

Providing a StackBlitz reproduction is the best way to share your issue.

I wanted to update this with 2 StackBlitzs that show the bug happening in safari and the fix.

Here is the StackBlitz with Angular updated to version 7 and using the Breakpoint Observer.

Here is the workaround I found using flex-layout.

The only difference in the code is the observable implementation. The flex-layout one works in safari while the breakpoint observer seems to still be broken.

Simply generate a new project using the cli.
run ng add @angular/material
run npm install @angular/cdk
run ng generate @angular/material:material-nav --name side-navigation
add <app-side-navigation></app-side-navigation> to app.component.html
run the app in safari and change the screen size to trigger the breakpoint observer

What is the use-case or motivation for changing an existing behavior?

This is just broken in Safari as far as I can tell.

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Latest versions as of now.

Angular: 7.1.3, Material: 7.1.1, Mac OS Sierra 10.12.6, TypeScript: ~3.1.6

Tested in Safari 11.0.2

Is there anything else we should know?

Was throwing this error on my larger project:

Error: ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value: 'mat-drawer-shown: true'. Current value: 'mat-drawer-shown: false'

This error doesn't usually get thrown with the above implementation.

The easiest way to see the bug is by paying attention to the button that opens the drawer. It should disappear when the breakpoint is not triggered and appear when the drawer is closed. Depending on the window size when first loading the app, the button may never appear or never disappear, whichever state it starts in.

@josephperrott josephperrott self-assigned this Aug 6, 2018
@josephperrott josephperrott added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label Aug 6, 2018
@RenatoFranca
Copy link

I was having the same problem. Here is the solution: https://blog.angularindepth.com/everything-you-need-to-know-about-the-expressionchangedafterithasbeencheckederror-error-e3fd9ce7dbb4

Now I'm using setTimeout:

breakpointObserver.observe(Breakpoints.Handset).subscribe(result => {
  if (result.matches) {
    setTimeout(() => {
      // content
    });
  }
});

In the end, this is not a bug. It's just the right behavior of Angular.

@mmalerba
Copy link
Contributor

mmalerba commented Aug 9, 2018

This appears to be something wrong in Angular animations. I made a repro that shows the issue still happening after removing the styles and start/end listeners from the animation, then working properly after I remove the animation completely. https://github.com/mmalerba/material2/tree/12479-repro @matsko

@Trent-Matthias
Copy link
Author

Trent-Matthias commented Aug 30, 2018

An update on this, I believe the issue is in the BreakpointObserver. The new StackBlitz I created uses the flex layout API instead to get media changes. I push that result to the isHandset$ Observable and without changing the template it works in Safari.
@mmalerba

After trying this out in my larger project the error in my first post still gets thrown, but everything seems to work despite that (the button appears and toggles the sidenav). The error only gets thrown when the drawer opens after a breakpoint check.

https://stackblitz.com/edit/angular-material2-issue-uulez4

@jbojcic1
Copy link
Contributor

jbojcic1 commented Sep 15, 2018

I also think the issue is with BreakPointObserver. I don't use sidenav at all and have this problem. Using setTimeout like @RenatoFranca has suggested fixed it for me. I guess it happens only on Safari due to the differences in handling micro and macro tasks.

@Trent-Matthias
Copy link
Author

Work around for ExpressionChangedAfterItHasBeenCheckedError for async pipe usage is to add tap(() => this.changeDetectorRef.detectChanges()) after returning the media query result. Would rather not have to mark for change detection though (and only for Safari), but it at least fixes my issue.

My whole observable composed with rxjs looks like this

private _isHandset$: Observable<boolean> = this.media.asObservable().pipe( map( () => this.media.isActive('xs') || this.media.isActive('sm') || this.media.isActive('lt-md') ), tap(() => this.changeDetectorRef.detectChanges()), distinctUntilChanged(), takeUntil(this.destroy$) );

@Trent-Matthias
Copy link
Author

I wanted to update this with 2 stackblitzs that show the bug happening in safari and the fix.

Here is the stackblitz with Angular updated to version 7 and using the Breakpoint Observer.

Here is the workaround I found using flex-layout.

The only difference in the code is the observable implementation. The flex-layout one works in safari while the breakpoint observer seems to still be broken.

I've updated the issue with this information, I believe the problem is in the breakpoint observer implementation.

@Trent-Matthias Trent-Matthias changed the title Sidenav issues in safari with breakpoints. Dec 19, 2018
@josephperrott josephperrott removed their assignment May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: cdk/layout area: material/sidenav P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
6 participants