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(core): improve use of breakpoint priorities #955

Merged
merged 1 commit into from Dec 30, 2018

Conversation

ThomasBurleson
Copy link
Contributor

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

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
Copy link
Member

@CaerusKaru CaerusKaru left a comment

Choose a reason for hiding this comment

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

LGTM

@CaerusKaru CaerusKaru added pr: lgtm This PR has been approved by the reviewer pr: merge ready This PR is ready for the caretaker to presubmit and merge and removed in-progress pr: needs cleanup/tests labels Dec 30, 2018
@CaerusKaru CaerusKaru added this to Pending in API Fixes via automation Dec 30, 2018
@CaerusKaru CaerusKaru added this to the 7.0.0-beta.23 milestone Dec 30, 2018
@ThomasBurleson ThomasBurleson merged commit d57b293 into master Dec 30, 2018
API Fixes automation moved this from Pending to Done Dec 30, 2018
@CaerusKaru CaerusKaru deleted the thomas/fix-bp-priority branch December 30, 2018 03:44
@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 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes pr: lgtm This PR has been approved by the reviewer pr: merge ready This PR is ready for the caretaker to presubmit and merge
Projects
API Fixes
  
Done
3 participants