Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Getting started on development #9

Closed
scarroll32 opened this issue Aug 30, 2018 · 5 comments · Fixed by #10
Closed

Getting started on development #9

scarroll32 opened this issue Aug 30, 2018 · 5 comments · Fixed by #10

Comments

@scarroll32
Copy link

Hi @adrienpoly this is a great package and good to see Stimulus-based packages arriving. We are using it in crowdAI.

This is more of a general how-to than a specific bug, I hope that is OK.

I think there is a bug around the 24 hour time configuration. In any case I can't make it work on crowdAI. I thought I could debug the package, in the way that one can debug a Rubygem. So far, I've tried this:

  1. Forked the repo
  2. Tried to add it from the fork
yarn add git+ssh//git@github.com:https://github.com/seanfcarroll/stimulus-flatpickr

But in fact this is not ideal. It would be better to be able to work locally on the dev environment.

I was wondering if you might have some ideas on how to develop with a local copy of the package?

@adrienpoly
Copy link
Owner

Hello @seanfcarroll

Understanding 100% your feeling as I when through this while developing this package. I have more of Ruby/Rails background....

What I have done so far in order to test local development is to use the npm linkfeature. This article got me on the right track: https://medium.com/@alexishevia/the-magic-behind-npm-link-d94dcb3a81af

This allows you to make a change in your local package and those changes are immediately visible in your test project.

After doing a change remember to build the package yarn build or to have it build continuously for every change with yarn dev

As a sum up (I ll try to make it better as a readme)

  1. fork the repo
  2. clone locally
  3. branch
  4. in the package directory npm link
  5. in the test project directory npm link stimulus-flatpickr (at this point if you open your node_module dir you should see the stimulus-flatpickr directory with a link icon instead of a standard directory icon
  6. make your change and build yarn buildor yarn dev
  7. in your test project make sure webpack rebuilds and refresh your page to see changes

I am currently working on a test suit should be up and running by end of next week. This also make it quite convinient once you are familiar with test to debug and develop the package. Goal is to have a quite good test coverage to release a 1.0.0...

Also recently I have published a new version of the package with a beta tab on npm. This version adds undocumented options from flatpickr. I don't think this is your issue as the 24h tag is part of the documented options.

Don't hesitate to describe the bug your are seeing too, if I can help will try

@scarroll32
Copy link
Author

Hi @adrienpoly thanks for the very informative answer. I will try it out and see if I can isolate the issue, the basic problem is the flatpickr_time_24hr: true setting seems to be ignored. I notice it is slightly irregular due to it being a time_24 camelcase, and perhaps there is something there.

https://github.com/crowdAI/crowdai/blob/master/app/views/challenges/form/_challenge_round_fields.html.erb#L75-L79

In any case I will follow your instructions, and maybe do a PR (unless the problem is at my end). I'm happy to also do a PR with these instructions if helpful. Cheers

@adrienpoly
Copy link
Owner

you are probably right about the camelcase issue

as a temp fix you can also set it as a default value for all your datepicker in your controller like this

connect() {
   //global options
    this.config = {
      enableTime: true,
      time_24hr: true
    };
    //always call super.connect()
    super.connect();
  }

This is nice for global options so that you don't need to pass them everytime you create a date field in your rails app

@adrienpoly
Copy link
Owner

I have started to add a little test example built with parcel bundler if you want to try it see this #10.
need to document it. a bit in a rush now will work on it later today/tomorrow

@adrienpoly
Copy link
Owner

time_24hr issue is now tracked here #11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants