AbpDatetimePicker - Expose _format for edit

[ndzeller]

Hello @ghiscoding ,

Very nice work on the plugins. Regarding the AbpDatetimePickerCustomElement, could the _format variable be exposed for edit? I noticed that the only place that it is set in the bind() method. I need to be able to change the format on the fly.

Thanks!

[ghiscoding]

PR (Pull Request) are welcome, if you can change the code that would be great. Also note that I'm currently in vacations and won't do any coding for couple weeks. Thanks

[ghiscoding]

After seeing your PR, I think that I misunderstood your question. I don't have the format or any other options exposed, that is by design. Every options available are exposed through the option.bind. Aren't you able to change the format on the fly with option.bind? I never tried, but don't see why this wouldn't work

EDIT After looking at the code, I can see that what is missing is actually optionChanged and I would rather include that instead of a formatChaged. I don't want to expose any options and it is by design, to remove confusions, everything should be in the option.bind. I don't want to have 2 ways to pass a format as I said, this would add confusion.

[ndzeller]

I had been trying to use option.bind and that created a buggy UX that didn't work correctly most of the time (ex. if one changed option.bind it would still use the original format after a date was selected). That is what prompted the issue creation.

If optionChanged is used instead of formatChanged does that fix the problem?

[ghiscoding]

I mentioned in my previous post that optionChanged is not in place, so it won't currently work. Tough I'm quite sure that implementing it would fix your problem and is the way to go. So I can't really accept your PR #45 for that reason, I would rather have another PR with optionChanged.

Also note that I'm still in vacation, until next week. If you have time to create a PR, that would be great, else it will probably take a few more weeks before I get time to get to it.

[ndzeller]

@ghiscoding , I've updated with the changes you suggested #45

[ghiscoding]

@ndzeller Oh cool, so I see you were able to do that in the optionChanged, however you've connected only the format property, right? I mean, what if the user changes another option, it won't do anything?

What if we try something like this instead:

  optionsChanged(newValue, oldValue) {
    if (newValue !== oldValue && newValue) {
      this.domElm.datetimepicker(this.options);
  }

I think that would re-create the picker with the new options, wouldn't that work? If so, it would work with any option changes.

I could be wrong on this, so please let me know if it make sense or not

[ndzeller]

Good point. I think that idea would work fine, although we would still need to handle updates to _format and model.

Something like this might be more complete for this feature and any other changes to options.

  optionsChanged(newValue, oldValue) {
    if (newValue !== oldValue && newValue) {
      let newFormat = newValue.format;
      if (newFormat && this._format !== newFormat && moment(this.model, newFormat).isValid()) {
        this._format = newFormat;
        this.model = moment(this.model, this._format).toDate();
      }
      this.domElm.datetimepicker(newValue);
    }
  }

What do you think @ghiscoding ?

[ghiscoding]

Yeah sure that looks good to me, would you mind trying to do the changes and possibly test with different option properties? For example, try changing the format and maybe minDate or maxDate

[ndzeller]

Sure, no problem.

They seem to work just fine :)

I'll finish up the commit and send a new PR

[ndzeller]

@ghiscoding , I've updated the code that we spoke about and updated the demo to include minDate and maxDate.

Tell me what you think when you get a chance! #45

EDIT: BTW, I had to use: this.domElm.data('DateTimePicker').options(newValue); instead of this.domElm.datetimepicker(newValue); because the latter was buggy and didn't work as expected.

[ghiscoding]

Right that make sense for the data('DateTimePicker'), I also use that for method call.

I'm still at work, but looking at your PR and I like it. It's nice, a date picker to change the date picker minDate haha great. 👍

That looks all good and is a really nice feature to add :) This is a personal project, so I'm not sure exactly when I will do the merge (after testing locally) but it should be by the end of this week. I do a lot of these in weekends.

[ndzeller]

Thanks @ghiscoding !

This feature is exactly what I need for my project so I'm glad you're on board for adding it :) I think other users will use this too.

BTW, great work on this project. You've written it very well, especially for being a personal project.

[ghiscoding]

@ndzeller Thanks a lot for the PR, that is a great feature and is now released NPM under version 1.3.0

Demo was also updated