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

feat(router): Add feature to support the View Transitions API #51314

Closed
wants to merge 2 commits into from
Closed

feat(router): Add feature to support the View Transitions API #51314

wants to merge 2 commits into from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Aug 9, 2023

The View Transitions API enables easy animations when transitioning between different DOM states. This commit adds an opt-in feature to the Router which runs the component activation and deactivation logic in the document.startViewTransition callback. If the browser does not support this API, route activation and deactivation will happen synchronously.

resolves #49401

@ngbot ngbot bot added this to the Backlog milestone Aug 9, 2023
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Aug 9, 2023
@atscott atscott force-pushed the viewtransitionrouter branch 3 times, most recently from 82c9144 to 68c20e5 Compare August 11, 2023 20:05
@atscott
Copy link
Contributor Author

atscott commented Aug 11, 2023

Updating risk to medium since the view transition adds a delay to where code was synchronous before. Exact timings in the Router have historically been fragile. It is technically possible to keep the code synchronous if the feature is not enabled, but avoiding "sometimes synchronous, sometimes asynchronous" branch in the code would be ideal.

@atscott atscott added the action: global presubmit The PR is in need of a google3 global presubmit label Aug 11, 2023
ajafff
ajafff reviewed Aug 14, 2023
View reviewed changes
packages/router/src/operators/activate_routes.ts Outdated Show resolved Hide resolved
packages/router/src/operators/activate_routes.ts Outdated Show resolved Hide resolved
packages/router/src/operators/activate_routes.ts Outdated Show resolved Hide resolved
packages/router/src/router_module.ts Show resolved Hide resolved
@atscott atscott force-pushed the viewtransitionrouter branch 4 times, most recently from 4c36be8 to e813133 Compare August 15, 2023 23:17
@atscott atscott force-pushed the viewtransitionrouter branch 5 times, most recently from bf34e47 to f275113 Compare August 28, 2023 15:51
@atscott
Copy link
Contributor Author

atscott commented Aug 29, 2023

Removing risk label since I ended up going with the sometimes sync, sometimes async behavior so that the default behavior is unchanged.

@atscott atscott force-pushed the viewtransitionrouter branch 2 times, most recently from e071790 to 0c58b90 Compare August 29, 2023 21:12
@atscott
Copy link
Contributor Author

atscott commented Aug 30, 2023

TGP shows no failures for the current approach. I ran a quick test to enable view transitions unconditionally and it appears that test failures are generally due to ZoneJS and the fact that it doesn't patch the startViewTransition API so the fakeAsync with flush and tick doesn't work. However, tests can instead use async/await along with the RouterTestingHarness to resolve these failures.

@atscott atscott added target: minor This PR is targeted for the next minor release and removed action: global presubmit The PR is in need of a google3 global presubmit labels Aug 30, 2023
@atscott atscott force-pushed the viewtransitionrouter branch 2 times, most recently from 8927fbe to 009b5e3 Compare August 30, 2023 21:25
@atscott atscott marked this pull request as ready for review August 30, 2023 22:26
jessicajaniuk
jessicajaniuk approved these changes Sep 1, 2023
View reviewed changes
Copy link
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

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

reviewed-for: public-api

AndrewKushnir
AndrewKushnir approved these changes Sep 1, 2023
View reviewed changes
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

Looks great, just a few minor comments.

packages/router/src/utils/view_transition.ts Outdated Show resolved Hide resolved
packages/router/src/navigation_transition.ts Outdated Show resolved Hide resolved
packages/router/src/navigation_transition.ts Outdated Show resolved Hide resolved
packages/router/src/provide_router.ts
* @returns A set of providers for use with `provideRouter`.
* @see https://developer.chrome.com/docs/web-platform/view-transitions/
* @see https://developer.mozilla.org/en-US/docs/Web/API/View_Transitions_API
* @experimental
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this is reflected on AIO, should we use @developerPreview instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure either but @jelbourn did mention this tag and we believe it's more appropriate given the state of the view transition API in general.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jelbourn It does appear that the @experimental tag doesn't show up on AIO (https://ng-dev-previews-fw--pr-angular-angular-51314-y5diongs.web.app/api/router/withViewTransitions)

packages/router/src/provide_router.ts Show resolved Hide resolved
AndrewKushnir
AndrewKushnir approved these changes Sep 1, 2023
View reviewed changes
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

Reviewed-for: public-api

@atscott atscott force-pushed the viewtransitionrouter branch 3 times, most recently from 5014f17 to e8b5d0b Compare September 5, 2023 21:00
@github-actions
Copy link

github-actions bot commented Sep 5, 2023 •
edited

Deployed aio for b3a240f to: https://ng-dev-previews-fw--pr-angular-angular-51314-y5diongs.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

@atscott
feat(router): Add feature to support the View Transitions API
5d70089 
The View Transitions API enables easy animations when transitioning between different DOM states. This commit adds an opt-in feature to the Router which runs the component activation and deactivation logic in the document.startViewTransition callback. If the browser does not support this API, route activation and deactivation will happen synchronously.

resolves angular#49401
@atscott atscott force-pushed the viewtransitionrouter branch 2 times, most recently from b822a0e to c92bcac Compare September 8, 2023 19:34
@atscott atscott added the action: merge The PR is ready for merge by the caretaker label Sep 11, 2023
@AndrewKushnir
Copy link
Contributor

This PR was merged into the repository by commit 73e4bf2.

@sp90 sp90 mentioned this pull request Sep 18, 2023
Closed
@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 Oct 12, 2023
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@atscott @ChellappanRajan
feat(router): Add feature to support the View Transitions API (angula…
9721f4b 
…r#51314)

The View Transitions API enables easy animations when transitioning between different DOM states. This commit adds an opt-in feature to the Router which runs the component activation and deactivation logic in the document.startViewTransition callback. If the browser does not support this API, route activation and deactivation will happen synchronously.

resolves angular#49401

PR Close angular#51314
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ajafff ajafff ajafff left review comments

@devversion devversion devversion approved these changes

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

@jessicajaniuk jessicajaniuk jessicajaniuk approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker aio: preview area: router detected: feature PR contains a feature commit target: minor This PR is targeted for the next minor release
Projects
None yet
Milestone
Backlog
Development

Successfully merging this pull request may close these issues.

Add support for View Transition API in Router
5 participants
@atscott @AndrewKushnir @devversion @ajafff @jessicajaniuk