Skip to content

refactor(ng-update): add links to migration guide in v9 hammer migration #17769

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

Merged
merged 1 commit into from
Nov 21, 2019
Merged

refactor(ng-update): add links to migration guide in v9 hammer migration #17769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 25 additions & 34 deletions src/material/schematics/ng-update/upgrade-rules/hammer-gestures-v9/gesture-config.template
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,61 +1,53 @@
/**
* Custom HammerJS configuration forked from Angular Material. With Angular v9,
* Angular Material dropped HammerJS as a dependency. This configuration was added
* automatically to this application because `ng update` detected that this application
* directly used HammerJS.
* automatically to this application because ng-update detected that this application
* directly used custom HammerJS gestures defined by Angular Material.
*
* If this application does not depend on the custom gestures originally defined by
* Angular Material, this file can be deleted.
* Read more in the dedicated guide: https://git.io/ng-material-v9-hammer-migration
*/

import {Injectable, Inject, Optional, Type} from '@angular/core';
import {HammerGestureConfig} from '@angular/platform-browser';
import {MAT_HAMMER_OPTIONS} from '@angular/material/core';

const SUPPORTED_CUSTOM_GESTURES = [
'longpress',
'slide',
'slidestart',
'slideend',
'slideright',
'slideleft'
];

/**
* Fake HammerInstance that is used when a Hammer instance is requested when
* HammerJS has not been loaded on the page.
* Noop hammer instance that is used when an instance is requested, but
* Hammer has not been loaded on the page yet.
*/
const noopHammerInstance = {
on: () => {},
off: () => {},
};

/**
* Adjusts configuration of our gesture library, Hammer.
* @deprecated No longer being used. To be removed.
* @breaking-change 10.0.0
* Gesture config that provides custom Hammer gestures on top of the default Hammer
* gestures. These gestures will be available as events in component templates.
*/
@Injectable()
export class GestureConfig extends HammerGestureConfig {
/** List of new event names to add to the gesture support list */
events = SUPPORTED_CUSTOM_GESTURES;

constructor(
@Optional() @Inject(MAT_HAMMER_OPTIONS) private _hammerOptions?: any) {
/** List of event names to add to the Hammer gesture plugin list */
events = [
'longpress',
'slide',
'slidestart',
'slideend',
'slideright',
'slideleft'
];

constructor(@Optional() @Inject(MAT_HAMMER_OPTIONS) private hammerOptions?: any) {
super();
}

/**
* Builds Hammer instance manually to add custom recognizers that match the
* Material Design spec.
*
* Our gesture names come from the Material Design gestures spec:
* https://material.io/design/#gestures-touch-mechanics
*
* More information on default recognizers can be found in Hammer docs:
* http://hammerjs.github.io/recognizer-pan/
* http://hammerjs.github.io/recognizer-press/
* Material Design specification. Gesture names originate from the Material Design
* gestures: https://material.io/design/#gestures-touch-mechanics
*
* More information on default recognizers can be found in the Hammer docs:
* http://hammerjs.github.io/recognizer-pan/
* http://hammerjs.github.io/recognizer-press/
* @param element Element to which to assign the new HammerJS gestures.
* @returns Newly-created HammerJS instance.
*/
Expand All @@ -66,7 +58,7 @@ export class GestureConfig extends HammerGestureConfig {
return noopHammerInstance;
}

const mc = new hammer(element, this._hammerOptions || undefined);
const mc = new hammer(element, this.hammerOptions || undefined);

// Default Hammer Recognizers.
const pan = new hammer.Pan();
Expand Down Expand Up @@ -95,11 +87,10 @@ export class GestureConfig extends HammerGestureConfig {
}

/** Creates a new recognizer, without affecting the default recognizers of HammerJS */
private _createRecognizer(base: Object, options: any, ...inheritances: Object[]) {
private _createRecognizer(base: object, options: any, ...inheritances: object[]) {
const recognizer = new (base.constructor as Type<any>)(options);
inheritances.push(base);
inheritances.forEach(item => recognizer.recognizeWith(item));
return recognizer;
}

}
11 changes: 8 additions & 3 deletions src/material/schematics/ng-update/upgrade-rules/hammer-gestures-v9/hammer-gestures-rule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,8 @@ export class HammerGesturesRule extends MigrationRule<null> {
'The HammerJS v9 migration for Angular Components detected that HammerJS is ' +
'manually set up in combination with references to the Angular Material gesture ' +
'config. This target cannot be migrated completely, but all references to the ' +
'deprecated Angular Material gesture have been removed.');
'deprecated Angular Material gesture have been removed. Read more here: ' +
'https://git.io/ng-material-v9-hammer-ambiguous-usage');
Copy link
Member Author

@devversion devversion Nov 21, 2019

Choose a reason for hiding this comment

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

I used git.io (which is an official Github service) since I don't want to show +150 chars for a perma-link. Happy to switch to something else if we can generate something like g.co/ng, or just use a real perma-link.

} else if (usedInTemplate && this._gestureConfigReferences.length) {
// Since there is a reference to the Angular Material gesture config, and we detected
// usage of a gesture event that could be provided by Angular Material, we *cannot*
Expand All @@ -188,7 +189,8 @@ export class HammerGesturesRule extends MigrationRule<null> {
'The HammerJS v9 migration for Angular Components detected that HammerJS is ' +
'manually set up in combination with references to the Angular Material gesture ' +
'config. This target cannot be migrated completely. Please manually remove ' +
'references to the deprecated Angular Material gesture config.');
'references to the deprecated Angular Material gesture config. Read more here: ' +
'https://git.io/ng-material-v9-hammer-ambiguous-usage');
}
} else if (this._usedInRuntime || usedInTemplate) {
// We keep track of whether Hammer is used globally. This is necessary because we
Expand Down Expand Up @@ -228,7 +230,8 @@ export class HammerGesturesRule extends MigrationRule<null> {
this.printInfo(
'The HammerJS v9 migration for Angular Components migrated the ' +
'project to keep HammerJS installed, but detected ambiguous usage of HammerJS. Please ' +
'manually check if you can remove HammerJS from your application.');
'manually check if you can remove HammerJS from your application. More details: ' +
'https://git.io/ng-material-v9-hammer-ambiguous-usage');
Copy link
Member Author

@devversion devversion Nov 21, 2019

Choose a reason for hiding this comment

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

This overall message still feels somewhat exaggerated. It's not always ambiguous, and only rarely (see case 1 in the guide). @jelbourn do you have any idea how we can rephrase this and make it not sound like it's always wrong?

Copy link
Member

@jelbourn jelbourn Nov 21, 2019

Choose a reason for hiding this comment

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

I think it's fine

}
}

Expand Down Expand Up @@ -852,6 +855,8 @@ export class HammerGesturesRule extends MigrationRule<null> {
'\n⚠ General notice: The HammerJS v9 migration for Angular Components is not able to ' +
'migrate tests. Please manually clean up tests in your project if they rely on ' +
(this.globalUsesHammer ? 'the deprecated Angular Material gesture config.' : 'HammerJS.')));
context.logger.info(
'Read more about migrating tests: https://git.io/ng-material-v9-hammer-migrate-tests');

if (!this.globalUsesHammer && this._removeHammerFromPackageJson(tree)) {
// Since Hammer has been removed from the workspace "package.json" file,
Expand Down