8000 feat(common): Allow passing ScrollOptions to ViewportScroller by atscott · Pull Request #61002 · angular/angular · GitHub
[go: up one dir, main page]
Skip to content

feat(common): Allow passing ScrollOptions to ViewportScroller #61002

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
Closed
wants to merge 1 commit into from
Closed

feat(common): Allow passing ScrollOptions to ViewportScroller #61002

wants to merge 1 commit into from

Conversation

atscott
Copy link
Contributor
@atscott atscott commented Apr 25, 2025

Adds ScrollOptions as an option second argument to ViewportScroller scrollTo* functions.

Part of changes needed to address #58258

@atscott
feat(common): Allow passing ScrollOptions to ViewportScroller
d6c3436 
Adds ScrollOptions as an option second argument to ViewportScroller
scrollTo* functions.

Part of changes needed to address angular#58258
@atscott atscott added the target: minor This PR is targeted for the next minor release label Apr 25, 2025
@angular-robot angular-robot bot added detected: feature PR contains a feature commit area: common Issues related to APIs in the @angular/common package labels Apr 25, 2025
@ngbot ngbot bot added this to the Backlog milestone Apr 25, 2025
AndrewKushnir
AndrewKushnir approved these changes Apr 25, 2025
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

@pullapprove pullapprove bot requested a review from kirjs April 25, 2025 21:58
devversion
devversion approved these changes Apr 29, 2025
View reviewed changes
Copy link
Member
@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewed-for: public-api

@atscott atscott added the action: merge The PR is ready for merge by the caretaker label Apr 29, 2025
kirjs
kirjs approved these changes Apr 29, 2025
View reviewed changes
Copy link
Contributor
@kirjs kirjs 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

packages/common/test/viewport_scroller_spec.ts
@@ -36,10 +36,15 @@ describe('BrowserViewportScroller', () => {
expect(() => scroller.setHistoryScrollRestoration('manual')).not.toThrow();
});

it('should not allow overwriting position with options', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

would be nice to have a test where scrollOption actually matter?

@mmalerba
Copy link
Contributor

This PR was merged into the repository by commit b7d3f3d.

The changes were merged into the following branches: main

@mmalerba mmalerba closed this in b7d3f3d Apr 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kirjs kirjs kirjs approved these changes

@devversion devversion devversion approved these changes

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

@mmalerba mmalerba Awaiting requested review from mmalerba

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: common Issues related to APIs in the @angular/common package 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.
5 participants
@atscott @mmalerba @kirjs @devversion @AndrewKushnir
0