dayClicked is not quite intuitive

[Mattkins]

Describe the bug

When you are trying to select a day, if the cursor moves AT ALL before mouseup, the dayClicked event does not fire. This is counterintuitive to how clicking works for end users. At the very least, as long as the mouseup happens inside the cell, dayClicked should probably still fire. This is causing a lot of frustration for our users since they are trying to rapidly select several days and it feels like the clicks just aren't happening.

Minimal reproduction of the problem with instructions

You can reproduce this in the selectable period demo. Just click in a cell and move the mouse before you let up on the click.

Screenshots

See demo.

Versions

• @angular/core: 7.2.15
• angular-calendar: 0.27.14
• Browser name and version: Version 77.0.3865.90 (Official Build) (64-bit)

[matts-bot]

Thanks so much for opening an issue! If you'd like me to give priority to answering your issue or would just like to support this project, then please consider sponsoring me

[mattlewis92]

How bizarre, I did some digging and it looks like the issue lies in hammerjs, as removing it from the demo and everything works as expected. I guess there's just some config in hammerjs that needs to be tweaked to get it to work, if you wanted to do some googling and see if there's a solution to this posted somewhere that would be awesome!

Edit: it looks like the pan event is firing which prevents the tap event from firing which is the cause of the issue: hammerjs/hammer.js#751

[Mattkins]

Would switching your click directive from tap to pressup be a viable fix?

[Mattkins]

Strangely, while playing around with dependencies, this seems to have fixed itself. I think a minor update on a dependency somewhere seems to have resolved this, as all I've done is run npm install. Maybe you can confirm with the demos that a dependency update is all that's required?

[mattlewis92]

Would switching your click directive from tap to pressup be a viable fix?

I tried that but it just never even triggered an event.

Strangely, while playing around with dependencies, this seems to have fixed itself. I think a minor update on a dependency somewhere seems to have resolved this, as all I've done is run npm install. Maybe you can confirm with the demos that a dependency update is all that's required?

I just tried updating all the dependencies but the issue is still present for me

[Mattkins]

That is so weird. Here's more detail if it might help us. I'm glad it's working on my build (verified on another machine, too), but I would really like to know what exactly I did to fix this.

1. I removed the hammerjs dependency from package.json and ran npm prune
2. I commented import 'hammerjs'; out in my src/main.ts file
3. I restarted the Angular dev server to make sure it recompiled, and tested to verify that I got the same effect you did by removing hammerjs. It began to work as expected (I could click on a day cell, move my cursor, release while still in the cell, and dayClicked would fire)
4. Not wanting to lose gesture support for Material, I re-added hammerjs to package.json and un-commented it in src/main.ts, then ran npm install
5. I observed that dayClicked was still working as expected.

I have not tested with a production build, yet. I will report back once I have.

[mattlewis92]

Maybe it's the way the library detects hammerjs, if it gets imported after angular-calendar then the feature detection won't work: https://github.com/mattlewis92/angular-calendar/blob/master/projects/angular-calendar/src/modules/common/click.directive.ts#L21

[Mattkins]

You are exactly right. I just moved the hammerjs import to the top of main.ts and the bug came back. Very strange that I somehow "fixed" it by just commenting and then un-commenting that import, and now import order has that effect where it didn't before. I even went back through my commits and made sure that I did not change the import order when I tried removing hammerjs, and there is no change to main.ts. Very, very weird.

Edit: I failed to make it clear that, after moving the hammerjs back down below the core imports, the bug went away again. Here's what my main.ts looks like right now:

import { platformBrowserDynamic } from '@angular/platform-browser-dynamic';

import { AppModule } from './app/app.module';
import { environment } from './environments/environment';

//Import HammerJS for gesture support in material
import 'hammerjs';

if (environment.production) {
  enableProdMode();
}

platformBrowserDynamic().bootstrapModule(AppModule)
  .catch(err => console.error(err));

[Mattkins]

I have verified that panend works if the cursor is moved, but it obviously does not work when the user makes a stationary click. It also still fires even if the user moves the cursor out of the cell before releasing the click, which is obviously counter-intuitive.

I think the logical solution involves a combination of tap, panend, and panmove, but I'm not quite sure how to restructure your click directive to accommodate all three events. I'm going to keep looking, but would appreciate any input you might have.

[mattlewis92]

So I'm kind of thinking that we can probably just remove hammerjs, it was originally added to fix this issue: #203 which I'm pretty sure has actually been solved by something else since. As of ios 11 there is no longer a 300ms click delay and the native (click) event behaves just like hammerjs's (tap) event. I will do some testing over the weekend, it would be great if we could just remove it as it causes so many issues haha.

[Mattkins]

I think you're right and probably Angular is the one abstracting the click delay out of the event. I can definitely say that just using the standard click event works great in the rest of our app.

[mattlewis92]

Please give 0.27.20 a go, it should work properly now 😄

[Mattkins]

Work great!