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

fxHide.lt-md (and others) is broken in 7.0.0-beta.20 #915

Closed
cfeltz34 opened this issue Dec 14, 2018 · 24 comments · Fixed by #916 or #923
Closed

fxHide.lt-md (and others) is broken in 7.0.0-beta.20 #915

cfeltz34 opened this issue Dec 14, 2018 · 24 comments · Fixed by #916 or #923
Assignees
Labels
bug has pr A PR has been created to address this issue P0 Critical issue that needs to be resolved immediately
Milestone

Comments

@cfeltz34
Copy link

Bug Report

What is the expected behavior?

When I use "fxHide.lt-md" attribute in a Angular project 7.x the component need to be hidden

What is the current behavior?

"fxHide" attribute hide the component
"fxHide.lt-md" attribute not hide the component with Angular 7.x
"fxHide.lt-md" attribute hide the component with Angular 5.4

What are the steps to reproduce?

https://stackblitz.com/edit/angular-piagbq?file=src%2Fapp%2Fapp.component.html

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

Angular 7.0.1 on Windows + Chrome (Version 70.0.3538.110 (Build officiel))

@CaerusKaru CaerusKaru added bug in-progress P0 Critical issue that needs to be resolved immediately labels Dec 14, 2018
@CaerusKaru CaerusKaru self-assigned this Dec 14, 2018
@CaerusKaru CaerusKaru added this to the 7.0.0-beta.20 milestone Dec 14, 2018
@CaerusKaru CaerusKaru pinned this issue Dec 14, 2018
@CaerusKaru
Copy link
Member

@cfeltz34 I'm working on this right now and we'll get a patch release out by EOD. This is just for fxHide/fxShow, correct?

@pscanlon1
Copy link

Yes.

Also would like to point out that I'm not crazy! :)

@jefbarn
Copy link

jefbarn commented Dec 14, 2018

Also seeing this with "fxLayoutGap.gt-sm"

@CaerusKaru
Copy link
Member

@jefbarn Yeah that tracks. I'm hunting down the fix now.

CaerusKaru added a commit that referenced this issue Dec 14, 2018
* Previously, we weren't registering the breakpoints when the
  media marshaller started up, which had been done by the old
  MediaMonitor. This corrects that

Fixes #915
@CaerusKaru
Copy link
Member

Here's the fix: #916

CaerusKaru added a commit that referenced this issue Dec 15, 2018
* Previously, we weren't registering the breakpoints when the
  media marshaller started up, which had been done by the old
  MediaMonitor. This corrects that

Fixes #915
CaerusKaru added a commit that referenced this issue Dec 15, 2018
* Previously, we weren't registering the breakpoints when the
  media marshaller started up, which had been done by the old
  MediaMonitor. This corrects that by registering the breakpoints
  in the marshaller

Fixes #915
@CaerusKaru
Copy link
Member

If anyone could test that it works using the nightly builds (npm i angular/flex-layout), that would be really helpful.

@CaerusKaru CaerusKaru added has pr A PR has been created to address this issue and removed in-progress labels Dec 15, 2018
@CaerusKaru
Copy link
Member

7.0.0-beta.21 has been released to address this issue

@CaerusKaru CaerusKaru changed the title fxHide.lt-md (and others) is broken in 7.0.0-beta.20 [solved] fxHide.lt-md (and others) is broken in 7.0.0-beta.20 Dec 15, 2018
@cfeltz34
Copy link
Author

cfeltz34 commented Dec 15, 2018 via email

@CaerusKaru CaerusKaru reopened this Dec 15, 2018
@CaerusKaru CaerusKaru changed the title [solved] fxHide.lt-md (and others) is broken in 7.0.0-beta.20 fxHide.lt-md (and others) is broken in 7.0.0-beta.20 Dec 15, 2018
@paulstelzer
Copy link

paulstelzer commented Dec 15, 2018

Can confirm that beta.21 is not working. For example this:

<div class="app-info-box" fxFlex="50" fxFlex.gt-sm="25">
...
</div>

fxFlex.gt-sm has no function:

chrome_2018-12-15_11-50-32

Stackblitz: https://stackblitz.com/edit/angular-3fnkgk

@CaerusKaru
Copy link
Member

I know this is an incredibly urgent issue, but at the moment I'm at a loss. The patch in 7.0.0-beta.21 added a test for this case explicitly, which is passing.

I'm still actively working on this, and we'll try to get a patch by next week.

@CaerusKaru CaerusKaru removed the has pr A PR has been created to address this issue label Dec 15, 2018
@CaerusKaru
Copy link
Member

The plot thickens as this exact template on our demo-app (which is built and compiled with the Angular CLI) works as expected. Now the question is why doesn't this work on StackBlitz? (or other apps if this isn't working for people using the CLI)

@cfeltz34
Copy link
Author

cfeltz34 commented Dec 15, 2018 via email

@CaerusKaru
Copy link
Member

@cfeltz34 I believe you! It's just so bizarre that our demo app works flawlessly with the same build artifacts.

@cfeltz34
Copy link
Author

cfeltz34 commented Dec 15, 2018 via email

@CaerusKaru
Copy link
Member

I tracked it down! And just when I thought I was losing the last remnants of my sanity. The issue with our demo app is that we have an ObservableMedia instance running that is doing something to properly register the overlapping breakpoints. Now we just have to make sure the MediaMarshaller is doing it too and it should solve the problem 🎉

@CaerusKaru CaerusKaru added the has pr A PR has been created to address this issue label Dec 16, 2018
@CaerusKaru
Copy link
Member

CaerusKaru commented Dec 16, 2018

This was an unbelievable PITA, but turned out to be super simple. There was a race condition in the new MediaMarshaller between registration of breakpoints and the activation for the observable. Essentially, the registration would fire off events, but the marshaller would not yet be listening. PR #921 fixes this and should be merged shortly. After that, we'll cut a new release, probably on Monday or Tuesday.

If it's urgent, you can use the nightly builds once it's been merged.

@CaerusKaru
Copy link
Member

@cfeltz34 Can you test this in your app? Please install the new version with npm i angular/flex-layout (note lack of @)

@CaerusKaru
Copy link
Member

Anecdotally with the StackBlitz provided, installing the latest nightly (enter https://github.com/angular/flex-layout-builds#master-232fc6e as the dependency) fixes this issue, so I'm going to unpin this and mark as resolved.

@CaerusKaru CaerusKaru unpinned this issue Dec 16, 2018
@paulstelzer
Copy link

paulstelzer commented Dec 16, 2018

@CaerusKaru Can confirm, it's working in my stackblitz, too :)

@cfeltz34
Copy link
Author

cfeltz34 commented Dec 16, 2018 via email

@xmlking
Copy link

xmlking commented Dec 17, 2018

Combine Uses of fxShow + fxHide not working for me as stated here

https://github.com/angular/flex-layout/blob/master/docs/documentation/fxShow-API.md

Env: @angular/core : 7.1.3 , angular/flex-layout: ^7.0.0-beta.21

    <div fxHide fxShow.gt-sm>test</div>

@paulstelzer
Copy link

Should fixed with beta.22 - or revert back to beta.19 if you cannot wait until the new beta release

@Trojaner
Copy link

I had the same issue, I can confirm that this issue is fixed with beta.22.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug has pr A PR has been created to address this issue P0 Critical issue that needs to be resolved immediately
Projects
None yet
7 participants