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

Datepicker: Add option for onUpdateDatepicker callback #1912

Merged
merged 2 commits into from
May 21, 2021

Conversation

patrick-vandy
Copy link
Contributor

Add a new option named onUpdateDatepicker that allows a custom callback to be provided. If provided, the callback is called at the end of $.datepicker._updateDatepicker.

Problem

Currently, there is not a good way to manipulate the DOM for the datepicker calendar. Using a common example, if you want to add a custom button, you have to do it in a callback on both beforeShow and onChangeMonthYear.

If using an inline datepicker, you also have to add the custom button in onSelect. Furthermore, If you call $('.my-date-picker').datepicker('refresh');, the custom button goes away and there is no callback option or event to use for adding it back.

Here is one of many stackoverflow.com posts on this problem.

This makes it difficult (sometimes impossible) to customize the datepicker calendar using the provided options, which leads to horrible hacks like this:

$.datepicker._base_updateDatepicker = $.datepicker._updateDatepicker;
$.datepicker._updateDatepicker = function (inst) {
    this._base_updateDatepicker(inst);
    // do custom stuff
};

Solution

Ideally, datepicker would trigger events like afterUpdate.datepicker and developers could then easily add event handlers to customize the behavior. However, since this isn't the current approach, I chose to add another callback option named onUpdateDatepicker that is always called (if defined) at the end of _updateDatepicker. This is a quick and easy solution that follows the current conventions of datepicker.

The following can now be used to customize the calendar in one place:

$('.my-date-picker').datepicker({
    onUpdateDatepicker: function(inst) {
        // do custom stuff
    }
});

Notes

I didn't see anything on how to update the documentation. If someone can please point me in the right direction, I will include updates for https://api.jqueryui.com/datepicker/.

Add a new option named onUpdateDatepicker that allows a custom callback
to be provided. If provided, the callback is called at the end of
$.datepicker._updateDatepicker.
Base automatically changed from master to main February 19, 2021 19:58
@mgol mgol requested a review from fnagel May 7, 2021 23:06
Copy link
Member

@fnagel fnagel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 by reading

The commit message "Datepicker: improve callbacks test for onUpdateDatepicker " should read "Datepicker: Improve callbacks test for onUpdateDatepicker"

@mgol Can we fix the commit message when merging?

@fnagel
Copy link
Member

fnagel commented May 14, 2021

Interesting PR! Seems like a valid use case and a proper approach.

Docs need a PR against https://github.com/jquery/api.jqueryui.com/blob/main/entries/datepicker.xml

@mgol I'm fine with this PR. What do you think? How to deal with the needed docs update?

Too bad the rewrite has never been released. It's using the widget factory and is much easier to extend :-(
https://github.com/jquery/jquery-ui/blob/datepicker/ui/widgets/calendar.js
https://github.com/jquery/jquery-ui/blob/datepicker/ui/widgets/datepicker.js

@patrick-vandy
Copy link
Contributor Author

Docs need a PR against https://github.com/jquery/api.jqueryui.com/blob/main/entries/datepicker.xml

@mgol I'm fine with this PR. What do you think? How to deal with the needed docs update?

@fnagel I cloned api.jqueryui.com and started updating datepicker.xml to include this option. However, I don't see how to provide example code for the option. For example, looking at the prevText option on that page I see code examples:

image

But looking at prevText in datepicker.xml I don't see where that's defined. I global searched the repo for prevText (and some other options with examples) but could not find where these examples are defined. Can you let me know how, as I'd like to include examples.

Make sure the custom element added by the onUpdateDatepicker callback still exists and is not duplicated after calling refresh and setDate.
@patrick-vandy
Copy link
Contributor Author

The commit message "Datepicker: improve callbacks test for onUpdateDatepicker " should read "Datepicker: Improve callbacks test for onUpdateDatepicker"

@fnagel I've updated the commit message 🙂

@mgol
Copy link
Member

mgol commented May 15, 2021

@patrick-vandy I did a bit of research and it seems code examples are generated based on metadata using one of the patterns from the https://github.com/jquery/grunt-jquery-content repository. This particular example is using this pattern:
https://github.com/jquery/grunt-jquery-content/blob/4534d3e6771cfea2d69a110d65cf33b88467a764/tasks/jquery-xml/entries2html-base.xsl#L594-L653

I'd start with making sure the new option is properly documented in the signatures section in entries/datepicker.xml and there's a chance the example will be generated automatically if you fill in all required fields.

@patrick-vandy
Copy link
Contributor Author

@fnagel @mgol Okay, I've opened jquery/api.jqueryui.com#334 to add this new option to the documentation. Let me know if there's anything else that needs to happen.

Also, I just realized on this PR I committed to master (though it looks like jQuery is using main now) instead of creating a feature branch on my fork. I can make a new branch off main and cherry pick my commits from this PR into a feature branch, then reopen a PR. Let me know if I should do that.

@fnagel
Copy link
Member

fnagel commented May 16, 2021

@patrick-vandy No need to change the source branch and the target branch has been changed from master to main by GH.

ui/widgets/datepicker.js Show resolved Hide resolved
ui/widgets/datepicker.js Show resolved Hide resolved
@fnagel fnagel merged commit a12c985 into jquery:main May 21, 2021
mgol pushed a commit to jquery/api.jqueryui.com that referenced this pull request Jun 18, 2021
@mgol mgol added this to the 1.13.0 milestone Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants