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

fix(breakpoint-observer): fix the breakpoint observer emit count and accuracy #15964

Merged

Conversation

TrevorKarjanis
Copy link
Contributor

@TrevorKarjanis TrevorKarjanis commented May 6, 2019

The breakpoint observer emits multiple and incorrect states when more than one query changes.
Debounce the observer emissions to eliminate the incorrect states and emit once.

References #10925 #15868

I have reproduced this in Chrome, Firefox, Safari, Edge, and IE. It occurs because the media query API fires an event when a query match changes. The breakpoint observer emits once for each event, updating the state of one query each time. Therefore, the observer may emit multiple times for one change and the reported intermediary states can be incorrect. I have reproduced up to five emissions for a single change.

https://stackblitz.com/edit/angular-6pmabc

matches: false
breakpoints:
{
"(max-width: 599.99px)": false,
"(min-width: 600px) and (max-width: 959.99px)": false,
"(min-width: 960px) and (max-width: 1279.99px)": false,
"(min-width: 1280px) and (max-width: 1919.99px)": false,
"(min-width: 1920px)": false
}
matches: true
breakpoints:
{
"(max-width: 599.99px)": false,
"(min-width: 600px) and (max-width: 959.99px)": true,
"(min-width: 960px) and (max-width: 1279.99px)": false,
"(min-width: 1280px) and (max-width: 1919.99px)": false,
"(min-width: 1920px)": false
}

PR #11007 attempted to resolve this issue. However, the issue still reproduces. My understanding is that the AsapScheduler uses a microtask which fails to debounce the media query events. Using the AsyncScheduler resolves the issue. AsyncScheduler uses a timer, setInterval, that may technically be slower on the first bounce but is more efficient across multiple.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label May 6, 2019
@TrevorKarjanis TrevorKarjanis changed the title fix<breakpoint-observer>: fix the breakpoint observer emit count and accuracy fix(breakpoint-observer): fix the breakpoint observer emit count and accuracy May 6, 2019
@TrevorKarjanis TrevorKarjanis force-pushed the fix-breakpoint-emissions branch 2 times, most recently from 72128f5 to 4ba5ead Compare May 7, 2019 20:23
…accuracy

The breakpoint observer emits multiple and incorrect states when more than one query changes.
Debounce the observer emissions to eliminate the incorrect states and emit once.

References angular#10925
@TrevorKarjanis TrevorKarjanis force-pushed the fix-breakpoint-emissions branch from 4ba5ead to 68b85ed Compare May 21, 2019 21:31
@josephperrott josephperrott self-assigned this Jun 3, 2019
Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@josephperrott josephperrott added target: patch This PR is targeted for the next patch release pr: lgtm action: merge The PR is ready for merge by the caretaker P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels Jun 3, 2019
@andrewseguin andrewseguin merged commit bb66df7 into angular:master Jun 26, 2019
@TrevorKarjanis TrevorKarjanis deleted the fix-breakpoint-emissions branch June 26, 2019 15:53
andrewseguin pushed a commit that referenced this pull request Jul 2, 2019
…accuracy (#15964)

The breakpoint observer emits multiple and incorrect states when more than one query changes.
Debounce the observer emissions to eliminate the incorrect states and emit once.

References #10925
@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 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants