adrienpoly / stimulus-flatpickr

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

Should be able choose which element the controller binds flatpickr to #35

Closed swrobel closed 5 years ago

swrobel commented 5 years ago

Perhaps this is my lack of understanding in stimulus, but the behavior seems strange.

div data-controller="flatpickr"
  div data-target="flatpickr.selection"
  = text_field_tag :start_date, nil, class: 'form-control', data: { controller: 'flatpickr' }

flatpickr_controller.js:

export default class extends Flatpickr {
  static targets = ['selection']

  change(selectedDates, dateStr, instance) {
    console.log(this.hasSelectionTarget)
  }
}

Upon selecting a date, console shows false. However, if I add flatpickr_wrap: true to my config in the view, then the console shows true when selecting a date. I don't understand why as the generated HTML looks very similar.

swrobel commented 5 years ago

After playing with this further, it seems that the issue is that connect() always expects this.element to be the input to bind flatpickr to, so I can't have a parent div that defines the controller with an input inside of it - instead flatpickr gets initialized twice. For now, I've solved this by overriding connect() like so:

  connect() {
    this._initializeEvents();
    this._initializeOptions();
    this._initializeDateFormats();

    this.fp = flatpickr(this.hiddenInputTarget, {
      ...this.config
    });

    this._initializeElements();
  }

I could work on a PR to make the target element configurable, if that seems like a reasonable course of action & would be welcome. Let me know...

adrienpoly commented 5 years ago

Thanks @swrobel for looking into this. Yes currently the design was to have the controller at the level of the input field. Would love to see on PR as you suggest to make this configurable.

PS: I am away right now from my dev environment. I am planining to completly review the test suite and fix a few issues strating mid august. So with regards to the CI don't worry too much at this point if some tests are failling as it seems that on Circle CI I have some racing conditions (local tests should be ok)

Nks commented 5 years ago

Follow up the issue. This is really actual. Thanks.

tabuna commented 5 years ago

Hi, I ran into the same problem. Any news

adrienpoly commented 5 years ago

ok @Nks @tabuna do you have any suggestions for the API for such a feature?

one way of doing this would be to have a reserved target :

<div data-controller="flatpickr" >
  <input data-target="flatpickr.instance">
</div>

instance could be a name for this reserved target, alternative suggestions?

another way would be to specify an id:

<div data-controller="flatpickr" data-flatpickr-element-id="some-id" >
  <input id="some-id>
</div>
tabuna commented 5 years ago

I do not have full competency in this matter. But the first option using instance, looks more attractive to me.

Because in working with stimulus you rely a little on unique element identifiers. At the same time, html looks complete. With the ability to specify a field for input in a non-controlled.

adrienpoly commented 5 years ago

ok so the idea would be as follow

during connect if this.hasInstanceTarget then I attach the flatpick to this target if not then it is the same business as usual and the Flatpickr instance is attached to this.element

ok I ll try to push something in beta tonight

Nks commented 5 years ago

e.g. right now we have problem with clearing the field after we allow to use time. By default flatpickr doesn't clear it on delete the value and put the value in any case. One of the option solve this - use custom template but with the current logic which always using input element this is not possible. So, instead of this the hack where I reimplemented connect method just to pass correct element to the flatpickr instance I wonder use custom element or current element when custom element is not exists or set.

eg.

import StimulusFlatpickr from "stimulus-flatpickr";
import rangePlugin from 'flatpickr/dist/plugins/rangePlugin';
import 'flatpickr/dist/l10n';

export default class extends StimulusFlatpickr {
    connect() {
        const element = this.element.querySelector('input');
        this._initializeEvents();
        this._initializeOptions();
        this._initializeDateFormats();

        this.fp = flatpickr(element, {
            ...this.config
        });

        this._initializeElements();
    }

    initialize() {
        const plugins = [];
        if (this.data.get('range')) {
            plugins.push(new rangePlugin({input: this.data.get('range')}));
        }

        this.config = {
            locale: document.documentElement.lang,
            plugins,
        };
    }

    clear() {
        this.fp.clear();
    }
}

And html is:

<div data-controller="fields--datetime" class="input-group">
            <input type="text" placeholder="Select Date.." autocomplete="off">
                <div class="input-group-append bg-white border-left-0">
                    <a class="input-group-text bg-transparent" title="clear" data-action="click->fields--datetime#clear">
                        <i class="icon-cross"></i>
                    </a>
                </div>
        </div>

Here the official example: https://flatpickr.js.org/examples/#flatpickr-external-elements

adrienpoly commented 5 years ago

I have publish a beta version 1.2.0-0 with the ability to specify using a target which element the controller binds flatpickr to

currently undocumented but here is a quick example of the html structure

<div data-controller="flatpickr" >
  <input data-target="flatpickr.instance">
</div>

let me know if it solves your use case

tabuna commented 5 years ago

let me know if it solves your use case

I installed the version 1.2.0-1 made such a controller:

import Flatpickr from 'stimulus-flatpickr';
import rangePlugin from 'flatpickr/dist/plugins/rangePlugin';
import 'flatpickr/dist/l10n';

export default class extends Flatpickr {
    /**
     *
     */
    initialize() {
        const plugins = [];
        if (this.data.get('range')) {
            plugins.push(new rangePlugin({ input: this.data.get('range') }));
        }

        this.config = {
            locale: document.documentElement.lang,
            plugins,
        };
    }
}

Also added the following html code:

<div
    class="input-group flatpickr-input"
    data-controller="fields--datetime"
    data-fields--datetime-enable-time="true"
    data-fields--datetime-time-24hr="false"
    data-fields--datetime-allow-input="true"
    data-fields--datetime-date-format="Y-m-d H:i:S"
    data-fields--datetime-no-calendar="false">

    <input type="text"
           placeholder="Select Date.."
           class="form-control"
           name="open"
           value=""
           autocomplete="off"
           data-target="fields--datetime.instance">
</div>

This did not work for me, the selected value is no longer applied to the field. Any help.

adrienpoly commented 5 years ago

Hello

sorry I didn't put the correct bundles in the npm package. I published a new version 1.2.0-2 with the correct bundles

Here is a little demo https://codesandbox.io/s/amazing-ritchie-b710u?fontsize=14&hidenavigation=1&theme=dark

hope this fixes it for you

tabuna commented 5 years ago

Hi @adrienpoly, just tried updating the version and it worked for me. Thank you very much, I look forward to when I can use this in a stable version.

adrienpoly commented 5 years ago

@Nks I have updated the example with the { wrap: true } option from Flatpickr https://codesandbox.io/s/heuristic-torvalds-65w3r?autoresize=1&fontsize=14&hidenavigation=1&theme=dark

If I understood correctly this is what you tried to achieve?

I will document those changes for the custom element and publish the official 1.2.0 within the week.

Nks commented 5 years ago

@adrienpoly thank you! Yes, I used it like @tabuna did and it works for me too. Thanks again for fast updates! :)

adrienpoly commented 5 years ago

1.2.0 is out 🚀

here is a codesandbox to illustrate the custom elements https://codesandbox.io/embed/nifty-wind-yqvt3?fontsize=14&hidenavigation=1&theme=dark