fix: #111

[mesqueeb]

fixes #111

I believe this should fix #111

edit: tested

[mesqueeb]

I tested via the example vite app and confirm the fix works.

[hunterliu1003]

Hi @mesqueeb,

Change mousedown to click event will break the two unit test cases (see report):

• VueFinalModal.vue › default props › clickToClose: true
• VueFinalModal.vue › events › all events

To fix this two errors, we can modify in VueFinalModal.spec.js

wrapper.find('.vfm__container').trigger('click')

to

 wrapper.find('.vfm__container').trigger('mousedown')

But I think the behavior of click and mousedown event is still different. I hope there is another solution that can keep using click event.

[mesqueeb]

I agree. We should also test for mobile.

[mesqueeb]

@hunterliu1003 I think though, if we test on mobile, and it works as expected, I don't see any reason why you would still prefer @click over @mousedown.

I think the behavior of click and mousedown event is still different.

I don't think so? 🤔

Perhaps the MouseEvent payload emitted on click is a bit different from the mousedown one, but since this is only for consumption internally in the dialog, it doesn't matter, right?

Do you have any examples or further clarification on how you think the behaviour might be different?

[hunterliu1003]

@mesqueeb

Do you have any examples or further clarification on how you think the behaviour might be different?

Using click event, modal will keep open until mouseup. So a situation I think of is when user accidently mousedown on the overlay, but maybe he doesn't want to close the modal currently. Then user could avoid modal close with moving mouse back to the modal container before the mouseup.

[mesqueeb]

@hunterliu1003 my last commit works with your requirement and mine.

Please see screencast here:
https://www.dropbox.com/s/cj13dfi2s9uknlw/Screen%20Recording%202021-06-12%20at%2017.18.37.mov?dl=0

Also added touch events, but this is something I have not yet tested.
So before we merge this PR we still need to make sure it works as expected on mobile.

[mesqueeb]

Sorry the screencast is not showing my clicks for whatever reason, so it's hard to see what I mean. 😅

[hunterliu1003]

@mesqueeb Thank you for fixing the issue quickliy.

I noticed that you using the stop event modifier on .vfm_container and .vfm__content, but this cause another issue about the compatibility with other lib that need click event bubble up, like choice.js.
you can checkout the PR #26: fix compatibility with choices.js.

I made some refactoring that was base on your idea but without using the stop event modifier and fixed the unit tests also in PR #129.

[hunterliu1003]

I create another PR #129 for this