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.

Sign up for GitHub

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

Jump to bottom

refactor(cdk/overlay): Reduce dependency on NgZone #28332

Merged
merged 2 commits into from Mar 8, 2024
Merged

refactor(cdk/overlay): Reduce dependency on NgZone #28332

merged 2 commits into from Mar 8, 2024

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Dec 27, 2023 •
edited

draft - Probably a breaking change and definitely not a good idea until afterNextRender runs only after ApplicationRef.tick. At the moment, this passes tests only because they're using fixtures and the fixtures are directly triggering change detection. However, in an application, the app component will refresh and trigger afterNextRender before the overlay views are refreshed.

@atscott atscott added the blocked This issue is blocked by some external factor, such as a prerequisite PR label Jan 4, 2024
@atscott
Copy link
Contributor Author

atscott commented Jan 4, 2024

blocked by angular/angular#52455

@atscott atscott mentioned this pull request Jan 20, 2024
Open
1 task
@atsjo
Copy link

atsjo commented Jan 20, 2024

eagerly awaiting this now that the blocker is resolved, will it also fix the tooltip positioning in zoneless?

@atscott
Copy link
Contributor Author

atscott commented Jan 20, 2024

eagerly awaiting this now that the blocker is resolved, will it also fix the tooltip positioning in zoneless

yes, this would fix the tooltip positioning in Zoneless. Note that the focus right now is finalizing the Zoneless implementation. There’s still plenty of work that needs to be done there before downstream fixes like this start to get more dedicated time and attention.

@atscott atscott marked this pull request as ready for review March 6, 2024 23:58
@mmalerba mmalerba removed the blocked This issue is blocked by some external factor, such as a prerequisite PR label Mar 7, 2024
atscott and others added 2 commits March 8, 2024 20:51
@atscott @mmalerba
fix(cdk/overlay): Allow cdk overlay to work with NoopNgZone
d3f9a22 
This updates the overlay implementation to use `afterNextRender` instead
of `ngZone.onStable` which does not emit at all in zoneless
applications. For zone-based applications, this means that the overlay
will be positioned immediately after the next ApplicationRef tick. This is slightly
different from `onStable`, which with ZoneJS can span multiple change
detection rounds.
@mmalerba
test: update API golden
8cda325
@mmalerba mmalerba added the action: merge The PR is ready for merge by the caretaker label Mar 8, 2024
@mmalerba mmalerba removed the request for review from jelbourn March 8, 2024 20:53
@mmalerba mmalerba added the target: major This PR is targeted for the next major release label Mar 8, 2024
@mmalerba mmalerba merged commit 7ec1e48 into angular:main Mar 8, 2024
24 of 27 checks passed
@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 Apr 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@crisbeto crisbeto crisbeto approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: cdk/overlay target: major This PR is targeted for the next major release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@atscott @atsjo @crisbeto @mmalerba