mymth / vanillajs-datepicker

A vanilla JavaScript remake of bootstrap-datepicker for Bulma and other CSS frameworks
MIT License
744 stars 154 forks source link

Please add support for Svelte #99

Open xenogenesi opened 2 years ago

xenogenesi commented 2 years ago

Thanks for this package!

I'm using it with Svelte (use directive) which is made also to support third-party vanilla javascript.

The problem I am having is with Svelte's bind:value attribute, this attribute should create a bidirectional binding between the input value and a variable, unfortunately at the moment when I select a date in the picker the value is set in the input but the binding is ignored (I'm not sure but I guess the problem arises because the value is set externally to the component and the Svelte compiler has no way to intercept the change).

Please see this repl

A solution that I have seen and used in other vanilla js code would be to expose two callbacks for the set and theget in the picker options, this way I could read write the variable directly and not the input value and since the implementation is in the component, the Svelte compiler would be able to manage the binding correctly.

{
// ...options
set: (inputEl, dateValue, pickerInstance) => { objVar[inputEl.name] = dateValue; },
get: (inputEl, pickerInstance) => objVar[inputEl.name],
}

Another minor problem is a warning that is presented during compilation, it is not a real problem but during development with the watch option it is printed at each update, it is a distraction.

[rollup-plugin-svelte] The following packages did not export their `package.json` file so we could
 not check the "svelte" field. If you had difficulties importing svelte components from a package,
 then please contact the author and ask them to export the package.json file.

- vanillajs-datepicker
mymth commented 2 years ago

I don't have experience with Svelte. So it would be appreciated if you correct me when I say something wrong.

I’m struggling to catch the point of the idea of the get/set callback options. Could you give me the references to the things that led you to the idea?

Although I haven't used Svelte, I have experience with its predecessor, Ractive.js, and wrote a decorator plugin for bootstrap-datepicker. (https://github.com/mymth/ractive-decorators-datepicker —The link to the demo is broken, but it can be seen at https://raw.githack.com/mymth/ractive-decorators-datepicker/v0.2.0/) I guess you need to do something similar to that plugin in the actionDateRangePicker2 function in your repl. So I’d like to know how the set/get callbacks are different from getting/setting date(s) with the getDate()/setDate() API (for DateRangePicker, getDates()/setDates()) methods inside the action function.

I noticed Svelte updates the variable bound to text field on every single key press. If you connect date picker to the variable so that Svelte keeps them in-sync, it will probably cause the date picker to redraw the UI on every key press parsing incomplete date string. Although no error occurs by parsing incomplete date string, user will see the calendar move around weirdly while typing (See https://mymth.github.io/vanillajs-datepicker/#/date-string+format for how irregular date strings are parsed.) For this reason, I have an impression that using Svelte’s 2-way binding with date picker’s input field may be a bit problematic.

As for the warning, my thoughts are the same as the people requesting the change of the issue 181 of rollup-plugin-svelte/issues/, to be honest. But I’m leaning to add it. Please allow me to take time to convince myself of the rationality of adding something that I don’t need in my package and is unnecessary for the packages which are not (or don’t include) a Svelte component. (I’m searching for the libraries that read other dependencies’ package.json field, but so far, rollup-plugin-svelte is the only one I could find.)

xenogenesi commented 2 years ago

Thanks for the reply!

Sorry, when I opened the issue I was in a bit of a rush, I got to spend some time looking at the documentation, I removed the bind attribute and now I use the changeDate event to update the variables, it seems to behave fine (repl updated)

I still have a problem probably with the date format (I'm using yyyy-mm-dd), it initializes correctly and opens the calendar with the date I put in the variable, but if I click one day it goes to another month / year, I'm still investigating.

I also had a problem with setDates and event listeners ending in loop, I had to remove the listeners first and re-add them later, maybe a setDates option would be useful to not fire the events? Are they generated when dates have not changed in value? If not, it could be due to the format problem itself.

Sorry I had not seen the issue 181 of the rollup plugin, I see that it continues in 192 which however is already closed, they asked for a PR but it was never produced, unfortunately the situation seems a bit stagnant on their side, it is the first time I see this problem and seeing their message I thought it was a common practice to export it. In the end it's just a warning, it's a bit annoying but that is, I can live with it.

xenogenesi commented 2 years ago

I confirm, the problem was date-fns's format() which wants MM for the month (mm is for minutes). I assume that the datepicker does not generate changeDate events when the dates that are set with setDates() do not change in value because now I no longer need to remove and reapply the listeners.

As far as I'm concerned, this issue can be closed, unless you want to keep it open for the warning issue.

Thank you!