Make custom events bubbling configurable

[Matu2083]

With the fix of #157 and PR #158 custom events are by default bubbling and cancelable. In some settings -- such as mine -- this can lead to unwanted behavior, specifically in cases where bubbling should be suppressed.

Suggestion: Extend the Datepicker class and its options to make event bubbling and event cancellation configurable and use that information when calling triggerDatepickerEvent (undifferentiated by type for now). I also would suggest to set the default of both options to 'false' to recover the 1.3.3 version behavior (and also to follow the defined default in the Event interface).

Happy to support. Just wanted know if there is a particular reason for changing the default behavior and to clarify the (potential) way forward.

[mymth]

The change was made because it's the behavior I originally planned (it's the same behavior as bootstrap-datepicker's events), but forgot to implement just by accident. Until #157, I wrongly thought that I programmed to make the events bubble. So, I'm not willing to go back to the unintended state caused by the mistake.

I think bubbling by default is more beneficial because it gives users flexibility on event handling, and they can also stop the bubbling just by calling event.stopPropagation().

For these reasons, even if I accept your suggestion and add the new option, I'll make it default to can-bubble.

Now, I have a question. Which do you think more work is needed to make, the new event config option in the library or something like this in your program?

import OriginalDatepicker from 'vanillajs-datepicker/Datepicker';

class Datepicker extend OriginalDatepicker {
  constructor(element, options = {}, rangepicker = undefined) {
    super(element, options, rangepicker);

    ['show', 'hide'].forEach((eventName) => {   
      element.addEventListener(eventName, (ev) => {
        ev.stopPropagation();
      });
    });
  };
}

[Matu2083]

Thanks for the quick answer and your proposed fix. Yes, your proposed solution does indeed work (not sure why I did not think of it in the first place). Yet, I have two more comments:

1. To me this fix is a major change with regards to the way Datepicker behaves (even if it was never intended to do so). So to some extend I consider this a "breaking" change.
2. I personally would consider having the event config option the cleaner and real solution (again happy to contribute taking into account your preferred defaults)

Thanks again

[mymth]

For 1, I apologize for the inconvenience. Noticing my not only mistake (in such basic stuff) but also misunderstanding, I might have been kind of in a panic and rushed too much to recover the mistake. However, there has never been any mention of the events' bubbling in the docs and thus, their not bubbling up has actually never been guaranteed. So, I feel like, "Sorry, but you decided to take a risk of using an undocumented gray-zone feature, didn't you?"

For 2, even if the events' constructor parameters are configurable via library's option, if the default is can-bubble, it seems pretty likely to me that most people just use stopPropagation() anyway because: a) libraries that have such option are very uncommon. (I don't know any.) Therefore, people may not even notice the option's existence until they read the docs carefully. b) calling stopPropagation() is not only a common technique, but also mandatory when preventing native DOM events such as click, keydown from bubbling up. Therefore, it's probably more intuitive, straightforward and familiar to most people. Also, I disagree with the thoughts that trying to solve a problem by adding a patch-like option is clean or a real solution. You can see my stance on this if you take a look at what I did in response to #103/#119 and #127, but in summary, it's generally a stopgap solution. Taking this approach time after time, the program gradually becomes hard to read. Thus, it's better to avoid unless there is no other way. For these reasons, it doesn't look to me that the new option is beneficial to many users.

I appreciate your offer, but I'm not interested in adding a feature that seems like most users don't care about. So, let's see if other people also want that option.

[Matu2083]

For 1: I agree on the gray-zone feature argument. It was never documented either way, yet I think it's fair to assume that if something follows the default behaviour for an Event and has done so for various version that this was the developers intent, even if not documented. For 2: I agree with your stance to keep only features that provide value to many users -- specifically if there is an easy solution to it as outlined by you above. Since I have not seen anyone else asking for that feature, I also do not see much sense in following up on it (unless there would be demand in which case my offer would still stand).

In any case, thanks for your time and the detailed responses.