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(ng-update): do not copy gesture config if only standard HammerJS events are used #17753

Merged
merged 1 commit into from Nov 20, 2019
Merged

refactor(ng-update): do not copy gesture config if only standard HammerJS events are used #17753

merged 1 commit into from Nov 20, 2019

Conversation

devversion
Copy link
Member

Currently the HammerJS v9 migration always copies the gesture config
if HammerJS events are used. This is not necessary for projects where
only standard HammerJS events are used. These events are provided
by the default hammer gesture config that comes with the HammerModule.

@devversion devversion added the target: patch This PR is targeted for the next patch release label Nov 19, 2019
@devversion devversion added this to the 9.0.0 milestone Nov 19, 2019
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Nov 19, 2019
@devversion devversion added the P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful label Nov 19, 2019
@devversion
refactor(ng-update): do not copy gesture config if only standard Hamm…
c5bdfc6 
…erJS events are used

Currently the HammerJS v9 migration always copies the gesture config
if HammerJS events are used. This is not necessary for projects where
only _standard_ HammerJS events are used. These events are provided
by the default hammer gesture config that comes with the `HammerModule`.
@devversion devversion force-pushed the refactor/hammer-v9-migration-differentiate-standard-events branch from bfde747 to c5bdfc6 Compare November 20, 2019 09:13
jelbourn
jelbourn approved these changes Nov 20, 2019
View reviewed changes
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

Commit message fixup: use fix instead of refactor so that this shows up in the changelog

@jelbourn jelbourn added merge: fix commit message When the PR is merged, rewrites/fixups of the commit messages are needed pr: lgtm action: merge The PR is ready for merge by the caretaker labels Nov 20, 2019
@jelbourn jelbourn merged commit b53e092 into angular:master Nov 20, 2019
@devversion
Copy link
Member Author

devversion commented Nov 20, 2019 •
edited

I intentionally used refactor since in the current state the migration did not generate anything broken. Mostly I wasn't too sure what we want as type here, but fix sounds good to me too.

@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 Dec 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jelbourn jelbourn jelbourn approved these changes

Assignees

@jelbourn jelbourn

Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement merge: fix commit message When the PR is merged, rewrites/fixups of the commit messages are needed P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful target: patch This PR is targeted for the next patch release
Projects
None yet
Milestone
9.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@devversion @jelbourn @googlebot