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

ObservableMedia: does not emit initial value in AOT mode #426

Closed
denver-HJS opened this issue Sep 20, 2017 · 23 comments · Fixed by #955
Closed

ObservableMedia: does not emit initial value in AOT mode #426

denver-HJS opened this issue Sep 20, 2017 · 23 comments · Fixed by #955
Assignees
Labels
bug has pr A PR has been created to address this issue P4 Low-priority issue that needs to be resolved
Milestone

Comments

@denver-HJS
Copy link

denver-HJS commented Sep 20, 2017

I've noticed a subtle difference in behavior in the ObservableMedia service when building with the CLI's AOT mode being on vs off.

I have a component property that dictates the responsive display of a section of content, and it's initialized via the subscription to the media change observable:

  constructor(private mediaQueryService: ObservableMedia) { }

  ngOnInit() {
      this.initializeFormOptions();
      this.initializeWatchers();
  }

  private initializeWatchers(): void {
    this.mediaWatcher = this.mediaQueryService
      .subscribe(mediaChange => {
        this.activeMediaViewport = mediaChange.mqAlias;
        this.lineEntryDisplayMode = this.calculateComponentDisplayMode(this.activeMediaViewport);
      });
  }

This works fine in non-AOT mode as the active mqAlias is available when the application loads. However, in AOT mode no value is emitted until a window resize event is triggered.

Since there is no direct query to the service available to get the current mqAlias, I am left with a less desirable brute force approach of querying individual possibilities using the isActive API method.

@alexfung888
Copy link

are you sure? Even without AOT, I don't always get an initial value of mqAlias. Not every time. I guess it is a race condition somewhere. Yes I ended up isActive()ing every breakpoint.

@ThomasBurleson ThomasBurleson added this to the v2.0.0-Beta.11 milestone Sep 26, 2017
@denver-HJS
Copy link
Author

Hmmm, actually I haven't had trouble with the initial value outside of AOT mode, but I did notice that it wasn't emitting a value after changing router states and returning back. That being the case, I ended up using isActive on every breakpoint as well.

Maybe a getActiveAlias API method that's limited to the 'xs | sm | md | lg | xl' aliases would be a useful addition? It looks like @ThomasBurleson has added this as a milestone for the upcoming beta version though 👍

@ThomasBurleson
Copy link
Contributor

@denver-HJS, @alexeagle - can you confirm these issues are STILL present with the nightly build ?

Installing nightly builds with npm

@denver-HJS
Copy link
Author

I'm not seeing any change using build @angular/[email protected]

With AOT:

13:30:35.071 vendor.bundle.js:80254 Angular is running in the development mode. Call enableProdMode() to enable the production mode.

Without AOT:

13:32:38.167 vendor.bundle.js:107377 Angular is running in the development mode. Call enableProdMode() to enable the production mode.
13:32:38.258 main.bundle.js:923 mediaChange --> (min-width: 600px) and (max-width: 959px)

Is that the latest nightly build?

@ThomasBurleson
Copy link
Contributor

@denver-HJS, Yes that is the latest build. Ok, we will track this one day and kill this bug!

@crysislinux
Copy link

crysislinux commented Oct 13, 2017

same as @alexfung888 Even without AOT, I don't always get an initial value. that happens when I resize window larger and smaller to trigger different breakpoints. After that, I never get the initial value.

Edit: I am using the nightly build

@ThomasBurleson
Copy link
Contributor

@crysislinux - We are aware of this issue and working on a fix.

@ThomasBurleson ThomasBurleson self-assigned this Oct 21, 2017
@ThomasBurleson ThomasBurleson modified the milestones: v2.0.0-Beta.11, v2.0.0-beta.10 Oct 21, 2017
@ThomasBurleson
Copy link
Contributor

ThomasBurleson commented Oct 21, 2017

@crysislinux - using the @angular/cli v1.5.0-rc2 and next version of Flex-Layouts, I am not seeing any problems with ObservableMedia when a demo application is served with ng serve (which uses AOT by default).

Get the next version using yarn add angular/flex-layout-builds --save. Note next is the most recent fixes not yet released to npm.

@ThomasBurleson
Copy link
Contributor

When using the latest build from github.com/angular/flex-layout-builds, ng serve -aot builds and the ObservableMedia service is working fine.

screen shot 2017-10-22 at 3 00 46 pm

screen shot 2017-10-22 at 2 58 43 pm

See Flex-Layout Seed project

@ThomasBurleson
Copy link
Contributor

Closing as not-an-issue.

@mozgor
Copy link

mozgor commented Feb 12, 2018

I might be wrong, but it seems to be linked to cli - present or not. I run two versions of my application atm, one using cli, the other on webpack custom ; cli version does trigger mediaChange on initialisation, while non cli version does not. Both use the same version of angular/flex-layout, 2.0.0-beta.10-4905443.

It would be useful to have a workaround cleaner than isActive() but I guess it's all I have for now.

@fervanrijswijk
Copy link

fervanrijswijk commented Feb 21, 2018

I have a similar issue. When I maximize the window, the ObservableMedia does not emit a new value, running 'ng serve'

@ThomasBurleson
Copy link
Contributor

@CaerusKaru - can you investigate this... using PR 586 ?

@fervanrijswijk
Copy link

fervanrijswijk commented Feb 27, 2018

To be more accurate on the issue I'm experiencing:
maximize to 'md' does NOT trigger the observable
maximize to 'xl' does trigger the observable

@fervanrijswijk
Copy link

fervanrijswijk commented Feb 27, 2018

Another addition:

Binding on the window.resize event, and checking which breakpoint is active gives me the following result:

xs false
sm false
md false
lg false
xl false

@k-schneider
Copy link

k-schneider commented Mar 12, 2018

I'm seeing an issue as well. Using ng serve and I have this little bit of code in one of my components:

this.media.subscribe(a => {
  console.log(a);
});

This does not log until I resize the window. I'd expect that it would log immediately when the app launches?

Edit: After more testing I can see if the screen is small or x-small the value emits immediately. Anything greater (medium, large, x-large) does not.

@CaerusKaru
Copy link
Member

As an update to this issue, we're looking at migrating the CDK's Media engine and deprecating ObservableMedia. Until we make definitive action on this, issues like this one are on-hold. If a community member wants to submit a PR for this or other OM issues, we'd be happy to review.

@CaerusKaru CaerusKaru modified the milestones: v2.0.1-beta.10, Backlog Mar 13, 2018
@CaerusKaru CaerusKaru added can be closed? discussion Further discussion with the team is needed before proceeding labels Dec 15, 2018
@CaerusKaru CaerusKaru added P4 Low-priority issue that needs to be resolved and removed can be closed? discussion Further discussion with the team is needed before proceeding labels Dec 19, 2018
@CaerusKaru CaerusKaru modified the milestones: Backlog, 7.0.0-beta.23 Dec 19, 2018
@CaerusKaru
Copy link
Member

After the post-mortem on the beta20/beta21 issues, we've realized that a similar thing might be happening here. We're going to patch it in beta 23 (slated for Jan 2019 release).

@CaerusKaru CaerusKaru added the has pr A PR has been created to address this issue label Dec 21, 2018
ThomasBurleson added a commit that referenced this issue Dec 22, 2018
Use breakpoint priority as the only sorting/scanning mechanism;
used to ensure correct MediaChange event notifications.

* prioritize breakpoints: non-overlaps hightest, lt- lowest
  * consistently sort breakpoints ascending by priority
  * highest priority === smallest range
  * remove hackery with reverse(), etc.
* memoize BreakPointRegistry findBy lookups
* fix MatchMedia::observe() to support lazy breakpoint registration
* fix fragile logic in MediaMarshaller
* fix breakpoint registration usage
* clarify update/clear builder function callbacks
* fix MediaObserver breakpoint registration usage

Fixes #648, Fixes #426
ThomasBurleson added a commit that referenced this issue Dec 22, 2018
Use breakpoint priority as the only sorting/scanning mechanism;
used to ensure correct MediaChange event notifications.

* prioritize breakpoints: non-overlaps hightest, lt- lowest
  * consistently sort breakpoints ascending by priority
  * highest priority === smallest range
  * remove hackery with reverse(), etc.
* memoize BreakPointRegistry findBy lookups
* fix MatchMedia::observe() to support lazy breakpoint registration
* fix fragile logic in MediaMarshaller
* fix breakpoint registration usage
* clarify update/clear builder function callbacks
* fix MediaObserver breakpoint registration usage

Fixes #648, Fixes #426
ThomasBurleson added a commit that referenced this issue Dec 23, 2018
Use breakpoint priority as the only sorting/scanning mechanism;
used to ensure correct MediaChange event notifications.

* prioritize breakpoints: non-overlaps hightest, lt- lowest
  * consistently sort breakpoints ascending by priority
  * highest priority === smallest range
  * remove hackery with reverse(), etc.
* memoize BreakPointRegistry findBy lookups
* fix MatchMedia::observe() to support lazy breakpoint registration
* fix fragile logic in MediaMarshaller
* fix breakpoint registration usage
* clarify update/clear builder function callbacks
* fix MediaObserver breakpoint registration usage

Fixes #648, Fixes #426
ThomasBurleson added a commit that referenced this issue Dec 27, 2018
Use breakpoint priority as the only sorting/scanning mechanism;
used to ensure correct MediaChange event notifications.

* prioritize breakpoints: non-overlaps hightest, lt- lowest
  * consistently sort breakpoints ascending by priority
  * highest priority === smallest range
  * remove hackery with reverse(), etc.
* memoize BreakPointRegistry findBy lookups
* fix MatchMedia::observe() to support lazy breakpoint registration
* fix fragile logic in MediaMarshaller
* fix breakpoint registration usage
* clarify update/clear builder function callbacks
* fix MediaObserver breakpoint registration usage

Fixes #648, Fixes #426
ThomasBurleson added a commit that referenced this issue Dec 28, 2018
Use breakpoint priority as the only sorting/scanning mechanism;
used to ensure correct MediaChange event notifications.

* prioritize breakpoints: non-overlaps hightest, lt- lowest
  * consistently sort breakpoints ascending by priority
  * highest priority === smallest range
  * remove hackery with reverse(), etc.
* memoize BreakPointRegistry findBy lookups
* fix MatchMedia::observe() to support lazy breakpoint registration
* fix fragile logic in MediaMarshaller
* fix breakpoint registration usage
* clarify update/clear builder function callbacks
* fix MediaObserver breakpoint registration usage

Fixes #648, Fixes #426
ThomasBurleson added a commit that referenced this issue Dec 28, 2018
Use breakpoint priority as the only sorting/scanning mechanism;
used to ensure correct MediaChange event notifications.

* prioritize breakpoints: non-overlaps hightest, lt- lowest
  * consistently sort breakpoints ascending by priority
  * highest priority === smallest range
  * remove hackery with reverse(), etc.
* memoize BreakPointRegistry findBy lookups
* fix MatchMedia::observe() to support lazy breakpoint registration
* fix fragile logic in MediaMarshaller
* fix breakpoint registration usage
* clarify update/clear builder function callbacks
* fix MediaObserver breakpoint registration usage

Fixes #648, Fixes #426
ThomasBurleson added a commit that referenced this issue Dec 30, 2018
Use breakpoint priority as the only sorting/scanning mechanism;
used to ensure correct MediaChange event notifications.

* prioritize breakpoints: non-overlaps hightest, lt- lowest
  * consistently sort breakpoints ascending by priority
  * highest priority === smallest range
  * remove hackery with reverse(), etc.
* memoize BreakPointRegistry findBy lookups
* fix MatchMedia::observe() to support lazy breakpoint registration
* fix fragile logic in MediaMarshaller
* fix breakpoint registration usage
* clarify update/clear builder function callbacks
* fix MediaObserver breakpoint registration usage

Fixes #648, Fixes #426
ThomasBurleson added a commit that referenced this issue Dec 30, 2018
Use breakpoint priority as the only sorting/scanning mechanism;
used to ensure correct MediaChange event notifications.

* prioritize breakpoints: non-overlaps hightest, lt- lowest
  * consistently sort breakpoints ascending by priority
  * highest priority === smallest range
  * remove hackery with reverse(), etc.
* memoize BreakPointRegistry findBy lookups
* fix MatchMedia::observe() to support lazy breakpoint registration
* fix fragile logic in MediaMarshaller
* fix breakpoint registration usage
* clarify update/clear builder function callbacks
* fix MediaObserver breakpoint registration usage

Fixes #648, Fixes #426
ThomasBurleson added a commit that referenced this issue Dec 30, 2018
Use breakpoint priority as the only sorting/scanning mechanism;
used to ensure correct MediaChange event notifications.

* prioritize breakpoints: non-overlaps hightest, lt- lowest
  * consistently sort breakpoints ascending by priority
  * highest priority === smallest range
  * remove hackery with reverse(), etc.
* memoize BreakPointRegistry findBy lookups
* fix MatchMedia::observe() to support lazy breakpoint registration
* fix fragile logic in MediaMarshaller
* fix breakpoint registration usage
* clarify update/clear builder function callbacks
* fix MediaObserver breakpoint registration usage
* cleanup tests: excessive use of `async()`.

Fixes #648, Fixes #426.
@simeyla
Copy link

simeyla commented Jan 16, 2019

Note: The method behavior has changed for isActive()

I was doing something similar with the initial state (the 'startWith' trick), but when updating to the latest version all my initial states broke. Previously you would provide an alias, but now (which makes more sense) you need to pass the actual media query.

 mm.isActive(bp.alias)   // old
 mm.isActive(bp.mediaQuery)   // new

@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 P4 Low-priority issue that needs to be resolved
Projects
None yet
Development

Successfully merging a pull request may close this issue.