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(common): add injection token for default date pipe timezone #43611

Closed
wants to merge 1 commit into from
Closed

feat(common): add injection token for default date pipe timezone #43611

wants to merge 1 commit into from

Conversation

crisbeto
Copy link
Member

Adds a new injection token called DATE_PIPE_DEFAULT_TIMEZONE that allows for the default timezone for all DatePipe instances to be specified.

Fixes #43031.

@google-cla google-cla bot added the cla: yes label Sep 27, 2021
@crisbeto crisbeto changed the title feat(common); add injection token for default date pipe timezone feat(common): add injection token for default date pipe timezone Sep 27, 2021
@crisbeto crisbeto force-pushed the 43031/date-pipe-timezone branch from 77e82a8 to 54afbf0 Compare September 27, 2021 16:32
@crisbeto crisbeto marked this pull request as ready for review September 27, 2021 17:23
@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer area: common Issues related to APIs in the @angular/common package target: major This PR is targeted for the next major release labels Sep 27, 2021
@ngbot ngbot bot modified the milestone: Backlog Sep 27, 2021
petebacondarwin
petebacondarwin suggested changes Sep 27, 2021
View reviewed changes
Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

This is great! I love the use of @Optional to avoid any breaking change for direct users of the DataPipe.

@mary-poppins
Copy link

You can preview 77e82a8 at https://pr43611-77e82a8.ngbuilds.io/.
You can preview 54afbf0 at https://pr43611-54afbf0.ngbuilds.io/.

@crisbeto crisbeto force-pushed the 43031/date-pipe-timezone branch from 54afbf0 to 9d17447 Compare September 27, 2021 20:30
@crisbeto
Copy link
Member Author

The feedback has been addressed.

@mary-poppins
Copy link

You can preview 9d17447 at https://pr43611-9d17447.ngbuilds.io/.

petebacondarwin
petebacondarwin approved these changes Sep 30, 2021
View reviewed changes
Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

Ideally we should fix the type of the injected param, but if not possible then LGTM.

@crisbeto crisbeto force-pushed the 43031/date-pipe-timezone branch 2 times, most recently from 73c3c2a to f6ba84c Compare October 6, 2021 16:44
@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 6, 2021
@crisbeto crisbeto modified the milestones: Backlog, v13 Feature Freeze Oct 6, 2021
@crisbeto
Copy link
Member Author

crisbeto commented Oct 6, 2021

@alxhub the feedback has been addressed.

@crisbeto crisbeto force-pushed the 43031/date-pipe-timezone branch 2 times, most recently from 8b946d9 to bd83bce Compare October 6, 2021 18:06
@dylhunn
Copy link
Contributor

dylhunn commented Oct 6, 2021

Presubmit is passing: http://test/OCL:401278522:BASE:401291859:1633543923861:f2e6d38f

@crisbeto
feat(common): add injection token for default date pipe timezone
79ad47a 
Adds a new injection token called `DATE_PIPE_DEFAULT_TIMEZONE` that allows for the default timezone for all `DatePipe` instances to be specified.

Fixes angular#43031.
@crisbeto crisbeto force-pushed the 43031/date-pipe-timezone branch from bd83bce to 79ad47a Compare October 6, 2021 20:20
alxhub
alxhub approved these changes Oct 6, 2021
View reviewed changes
Copy link
Member

@alxhub alxhub 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

@dylhunn
Copy link
Contributor

dylhunn commented Oct 6, 2021

This doesn't need another presubmit run, since all changes since the last are docs-only

petebacondarwin
petebacondarwin approved these changes Oct 6, 2021
View reviewed changes
Copy link
Contributor

@petebacondarwin petebacondarwin 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 approved these changes Oct 6, 2021
View reviewed changes
Copy link
Contributor

@atscott atscott 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

@dylhunn
Copy link
Contributor

dylhunn commented Oct 6, 2021

This PR was merged into the repository by commit adf4481.

@dylhunn dylhunn closed this in adf4481 Oct 6, 2021
@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 Nov 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@petebacondarwin petebacondarwin

@atscott atscott

@alxhub alxhub

@IgorMinar IgorMinar

@jelbourn jelbourn

@jessicajaniuk jessicajaniuk

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit area: common Issues related to APIs in the @angular/common package cla: yes target: major This PR is targeted for the next major release
Projects
None yet
Milestone
v13 Feature Freeze
Development

Successfully merging this pull request may close these issues.

Feature Request: DatePipe timezone override
6 participants
@crisbeto @mary-poppins @dylhunn @petebacondarwin @atscott @alxhub