Components: MdSlider no animation in lazy loaded component

Created on 16 May 2017  Â·  18Comments  Â·  Source: angular/components

Bug, feature request, or proposal:

If you don't import MdSliderModule in a non-lazy loaded module, then the gesture(slide animation) is not present on the lazy loaded module.

What is the expected behavior?

MdSlider only needs to be imported in a component that uses it. Not the root when loaded only in a lazy loaded module.

What is the current behavior?

MdSlider has to be imported in the root when only used in lazy loaded module to get gestures. Slide animation.

What are the steps to reproduce?

Providing a Plunker (or similar) is the best way to get the team to see your issue.
Plunker template: https://goo.gl/DlHd6U

What is the use-case or motivation for changing an existing behavior?

Bug

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Angular 4.1.2
Chrome 58.0.3029.110
macOS
typescript 2.3.2
material beta 5

Is there anything else we should know?

Already fixed import order i.e. Can't drag md-slider #4278

P3 materiaslider

Most helpful comment

Can confirm, I wasn't able to get animations for slider+drag to work when MdSliderModule was only included in lazy loaded module. Adding MdSliderModule import to main app module temporarily resolves the lack of animation.

All 18 comments

Can confirm, I wasn't able to get animations for slider+drag to work when MdSliderModule was only included in lazy loaded module. Adding MdSliderModule import to main app module temporarily resolves the lack of animation.

Confirmed on 2.0.0-beta.8; none of the other "canon" solutions relating to HammerJS (import order, adding to .angular-cli.json) seem to resolve the lack of sliding – only importing in a non-lazy module does.

I can confirm that this issue still exists on latest version! (Both 5 and 6.0.0beta2). There's anyone looking on this?

I think this should at least be documented in the API until it's fixed, it was very annoying to find.

+1

+1

+1

Since this issue has devolved into the usual litany of "+1"s, I'm going to go ahead and try to dig this one up in a more positive way!

I noticed that the original Plunker that was provided for this issue no longer works (it was made over a year ago, after all), so I've gone ahead and re-made a commented and absolutely minimal reproduction of this issue, using the Stackblitz platform that the Material team seems to prefer now! That should at least ensure we have a working example for a while to come – System.import() always seemed dicey to use in paste bins anyway.

Now, for the meat: what can I do to assist with this? @mmalerba, I see you're assigned here, have you made any discoveries that could help point me in the right direction? Have you, @jelbourn? Any insight anyone can provide (i.e. suspect files, test results, etc) would be wonderful so I'm not just sticking my finger on the globe.

I want to help!

@DevVersion want to take a look?

I've looked into this issue and was able to reproduce the problem.

The lazy loading of Material components which depend on a custom GestureConfig does currently not work because the HammerGesturesPlugin is being created before our child lazy-loaded components can overwrite the HAMMER_GESTURE_CONFIG injection token (seen here)

Due to the fact that HammerGesturesPlugin is created once and therefore not recognizes the new custom gesture config, the default gesture config will be used to render the MatSlider or MatSlideToggle component.

Since such components depend on custom Hammer events, the events will never fire and the gestures won't work.

Workaround:

A workaround would be to manually specify the HAMMER_GESTURE_CONFIG in the _root app module_ that also imports the BrowserModule.

import {GestureConfig} from '@angular/material';

@NgModule({
  ...
  providers: [
    {provide: HAMMER_GESTURE_CONFIG, useClass: GestureConfig}
  ]
})
export class AppRootModule {}

Possible solutions:

  1. No longer depending on a custom HammerGestureConfig (needs investigation)
  2. Encouraging developers to manually specify the gesture config if lazy-loading is used
  3. Manually using the custom Hammer instance without using the HammerGesturesPlugin

@DevVersion I can confirm this workaround does indeed work beautifully! Well done. I've updated my StackBlitz to include a commented example of this workaround for anyone coming late to this issue.

So for the actual problem – this definitely sounds a bit more complicated than it looks on the surface, seems the issue comes down to architectural crossroads more than a bonafide 'bug'.

For the proposed solutions, here are my two cents:

  1. Which part needs investigation? If it's something along the lines of "how different is Material's gesture config from the default and where could it compromise, etc", I can help out here for sure.

    • Addendum: would this mean HammerGestureConfig would be removed entirely, or just left to users to implement?

  2. That's actually not a bad short term solution given the breadth of the issue, it certainly works like a charm and doesn't even seem too smelly.
  3. Where would this be done? Would that behaviour just be deferred to the EventManager, or somewhere different completely? Or up to user?

Thanks for all this great info.

@wosevision Glad the workaround works and doesn't feel to "smelly".

_Regarding_:

  • 1) We would need to figure out whether we could replace our custom (slide) Hammer events with the default (pan) events. Also we'd need to find a way to replace the (longpress) event.

  • 3) I thought about just injecting some "gesture" service that then delegates to our _custom_ HammerJS instance. We would have to deal with the HTMLElement's though.

These are just some approaches that came to mind at first glance.

Hmm.. okay, this is a good start. I'm going to look into what replacing those events would entail, I'll need to do some tests with switching event types out for one another.

I can't claim to fully understand proposition 2 still though, specifically the difference from the service in hammer_gestures.ts.. I'm still feverishly poking through these files though so it may click yet

The third proposal would be something like that:

```ts
class MatSlideToggle {

constructor(gesture: /* Sample name / MatGestureService) {
if (gesture.isEnabled()) {
gesture
.createHammerInstance(this._slideToggleDragContainerRef.nativeElement)
.on('slide', /
handle event */);
}
}
}
````

This way, we always have our _needed_ custom HammerJS instance and it would work better for lazy-loading.

Oh, I see! Okay, that makes perfect sense now, thanks. So the service would only ever be responsible for providing the behaviour on a case-by-case basis – the hammer instance would just get attached to each element that needs it.

That sounds like a pretty reasonable solution too, and possibly one that gets to the heart of the issue. The only compromise (as far as I can tell) would be ergonomics for users who choose to roll their own custom everything using Material's services, and I would hope that _they_'re at least used to expecting non-canon methods that look "internal-y". I personally wouldn't feel the loss if HammerGesturesPlugin disappeared tomorrow, but I obviously can't speak for everyone.

one application can have only one HAMMER_GESTURE_CONFIG.
material should not use it, otherwise developer can't use it anymore.

HammerJS has been removed, so that is no longer an issue 🎉

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._

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Miiekeee picture Miiekeee  Â·  3Comments

3mp3ri0r picture 3mp3ri0r  Â·  3Comments

vitaly-t picture vitaly-t  Â·  3Comments

kara picture kara  Â·  3Comments

crutchcorn picture crutchcorn  Â·  3Comments