search

adrienpoly / stimulus-flatpickr

A modest, yet powerful wrapper of Flatpickr 📆 for Stimulus
MIT License
415 stars 30 forks source link

Range plugin #36

Closed pedrocarmona closed 5 years ago

pedrocarmona commented 5 years ago

Hi Adrien,

Thank you for this lib. I am adding instructions to how to use the flatpickr's range plugin. Could also be added to the lib if you prefer.

Thank you, Pedro

adrienpoly commented 5 years ago

Thanks, @pedrocarmona for this documentation. I agree that the use of plugins is not yet correctly documented. I did put your work into a small Glitch project https://glitch.com/edit/#!/stimu-fpkr-range?path=src/controllers/datepicker_controller.js:18:3

My main concern is that to achieve this we are using functions with underscore prefix which are meant to be private and could, therefore, change in the future. I

35 with a suggestion from @swrobel to add a PR for selecting the element to add the datepicker could be a start of a solution to properly tackle this issue.

Bottom line, to fully support the range plugin it does require some extra work on the lib itself

swrobel commented 5 years ago

Thanks for the mention. After a short experiment with Stimulus, I've gone back to React probably won't be contributing to this codebase, unfortunately.

pedrocarmona commented 5 years ago

Hi Adrian, thanks for feedback. I agree with you :) If you want I can make a PR with configurable element, just like in the example.

In another topic, I started using stimulusjs recently and I really like you approach to save and apply flatpickr's config. I am copying this approach to use a private controller for slim_select, and it feels not DRY. I see that few parts could be extracted:

adrienpoly commented 5 years ago

@pedrocarmona about your suggestions, lots of thing will be largely simplified with the Value API please see https://github.com/stimulusjs/stimulus/pull/202

You can already test it by using "stimulus": "https://github.com/stimulusjs/dev-builds/archive/b8cc8c4/stimulus.tar.gz"

About your proposed PR go ahead. How would you describe the API? I was thinking that this could maybe be automatic if within the element there is both a rangeStart & rangeStop Target. Can we do in such case a dynamic import of the npm package?

adrienpoly commented 5 years ago

42 & #43 adds new functionalities to support range plugins. Thanks for taking the time to suggest some solutions in the meantime.