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

[747] Updates for jquery datepicker plugin and javascript includes cleanup #749

Conversation

jsavell
Copy link
Contributor

@jsavell jsavell commented Mar 1, 2024

This PR:

The latest, original version of the plugin appears to include all the modern jquery updates that were provided by the forked version along with additional improvements and fixes. I haven't found any issues testing these changes locally.

@jsavell jsavell changed the title [747] Updates for jquery datepicker plugin and javascript included cleanup [747] Updates for jquery datepicker plugin and javascript includes cleanup Mar 5, 2024
@andyp-uk andyp-uk requested a review from spaceisntsyntax March 8, 2024 22:02
@andyp-uk
Copy link
Contributor

@jsavell thanks for this. How much testing do we need to do? Can you please give us some guidance? Will these changes impact other modules apart from Resources?

@jsavell
Copy link
Contributor Author

jsavell commented Mar 21, 2024

Hi, this date picker plugin is shared across multiple/all modules, so the fix impacts/benefits all usages of the '.date-pick' class hook.

The javascript includes clean up is isolated to the resource.php file itself.

For testing, just verifying that the date picker pop ups still appears should be enough. If your installation wasn't affected by the DST bug, there shouldn't be any visible differences before/after.

In applying this PR to this 'ERM-63-modern-jquery-update-with-php8' branch, I did find a case where the date picker pop up wasn't applying properly: https://github.com/coral-erm/coral/blob/ERM-63-modern-jquery-update-with-php8/management/ajax_forms.php#L121

I tested this issue on the parent 'ERM-63-modern-jquery-update-with-php8' without my update, and the problem was already present. I also tested on the Coral Demo site and the problem was not present. I've opened a separate PR to address that issue.

@andyp-uk andyp-uk merged commit 169d9aa into coral-erm:ERM-63-modern-jquery-update-with-php8 Apr 4, 2024
@andyp-uk andyp-uk removed the request for review from spaceisntsyntax April 23, 2024 12:58
@andyp-uk andyp-uk added bug This is a bug (not an enhancement) refactor labels Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This is a bug (not an enhancement) refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants