[Drupal 9.x] Only load jQuery UI Datepicker when needed [#3246947]

Problem/Motivation

Every time a date field is visible, the jQuery UI Datepicker library is loaded. However, the datepicker option is not used all the time, espcially now that some browsers can show the datepicker without the library being enabled.

Instead of always loading the jQuery UI Datepicker, only load it when the datepicker option is enabled.

Steps to reproduce

Enable the claro theme, and install jquery_ui_datepicker and webform_ui
Go to a form build page
Click + Add element
Click Show preview
The jquery_ui_datepicker is now loaded, even though it is not used.

You can easily see that the jquery_ui_datepicker library is loaded, because of a Drupal core bug with jquery_ui (#3195764: Modules depending on jQuery UI have broken styling since Drupal 9.2). Buttons have the wrong textcolor and are therefore unreadable.

If the library would not be loaded, this bug would not occur.

Proposed resolution

Only load the library when the datepicker is enabled, and not always if jquery_ui_datepicker is enabled.

Remaining tasks

Create MR.

User interface changes

None.

API changes

None.

Data model changes

None.

Comment	File	Size	Author
#11	3246947-3.patch	2.48 KB	bartlangelaan
#11	6.1.x: PHP 7.4 & MySQL 8, D9.2.9 531 pass
#7	3246947-7.patch	5.27 KB	jrockowitz
#7	6.1.x: PHP 7.4 & MySQL 8, D9.2.9 531 pass
#6	3246947-6.patch	0 bytes	jrockowitz
#6
#4	3246947-4.patch	1.39 KB	jrockowitz
#4	6.x: PHP 7.3 & MySQL 5.7, D9.2 531 pass

Issue fork webform-3246947

Show commands

Start within a Git clone of the project using the version control instructions.

Add & fetch this issue fork’s repository

Or, if you do not have SSH keys set up on git.drupalcode.org:

Add & fetch this issue fork’s repository

3246947-only-load-jquery changes, plain diff MR !63
Check out this branch

About issue forks

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

1 November 2021 at 07:55

bartlangelaan created an issue. See original summary.

Comment #2

1 November 2021 at 08:14

bartlangelaan opened merge request !63

Comment #3

bartlangelaan

Dutch

Netherlands

CreditAttribution: bartlangelaan at iO commented 1 November 2021 at 08:19

Status:

Active

» Needs review

See MR !63.

Comment #4

jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commented 6 November 2021 at 11:15

File	Size
3246947-4.patch	1.39 KB
6.x: PHP 7.3 & MySQL 5.7, D9.2 531 pass

Here is a slightly different approach.

Comment #5

bartlangelaan

Dutch

Netherlands

CreditAttribution: bartlangelaan at iO commented 6 November 2021 at 11:50

We have jquery_ui_datepicker installed because it is a requirement of better_exposed_filters. (it will be removed in the future, see #3210945: Remove dependency on jquery_ui_datepicker)

However, we rarely use datepickers in webforms and of course on a lot of pages the bef module is not used. Loading the datepicker library even though it's not needed seems not logical - but I may be missing something.

It also causes a bug with Drupal 9.2 styling if Claro is enabled, and element previews are enabled. This is a core bug (#3195764: Modules depending on jQuery UI have broken styling since Drupal 9.2) but I won't know if it will be solved, because jQuery UI is phased out.

Not loading jQuery UI when it's not needed seems like a good thing to do. That's why I prefer !63 over your patch.

Comment #6

jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commented 2 December 2021 at 10:43

Title:

Only load jQuery UI Datepicker when needed

» [Drupal 9.x] Only load jQuery UI Datepicker when needed

File	Size
3246947-6.patch	0 bytes

I agree that we need to phase out the dependency. That attached POC patch isolated the jquery_ui_datepicker to a dedicated library.

The patch DOES NOT work, I think it is because of #3195764: Modules depending on jQuery UI have broken styling since Drupal 9.2

Since we are no longer supports Drupal 8 support, The patch should solution should only target Drupal 9.x.

Comment #7

jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commented 2 December 2021 at 10:44

File	Size
3246947-7.patch	5.27 KB
6.1.x: PHP 7.4 & MySQL 8, D9.2.9 531 pass

There was a typo in the patch. It should be working as expected. This approached gets us one step closer to removing support for the jquery UI datepicker.

Comment #8

jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commented 2 December 2021 at 10:46

Parent issue:

» #3249740: [META] Ensure webform compatibility with Drupal 9 only.

Comment #9

saschaeggi

he/him

CreditAttribution: saschaeggi as a volunteer and at Unic commented 2 December 2021 at 11:21

Status:

Needs review

» Reviewed & tested by the community

I was actually just including the patch from #4 before @jrockowitz now posted the new patch #7.
It fixed the issue we had with Drupal 9.2+ for us.
Patch from #7 works well.

Comment #10

jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commented 2 December 2021 at 12:13

Status:

Reviewed & tested by the community

» Needs review

Should we take this a step further and move all jquery_ui_datepicker code into a deprecated webform_ jquery_ui_datepicker.module?

This would be a bit of work because the date picker code is sprinkled throughout the webform's date elements.

The sequence to do this would be to

Create dedicated date picker test webforms and tests.
Create webform_ jquery_ui_datepicker.module
Move Date, DateList, and DateTime date picker code to the new module.
Move tests
Write update hook enable new module is the jquery_ui_datepicker.module is installed.

This is such a complex task, I would probably have to take the first crack at it.

Comment #11

bartlangelaan

Dutch

Netherlands

CreditAttribution: bartlangelaan at iO commented 5 December 2021 at 22:32

Issue summary:

View changes

File	Size
3246947-3.patch	2.48 KB
6.1.x: PHP 7.4 & MySQL 8, D9.2.9 531 pass

The issue is still not fixed for me. The datepicker library is loaded, even though I have not enabled the datepicker in a date element.

I have changed the issue summary so it includes a way to easily see if the library is loaded.

I have also attached a patch, which is exactly the same as MR!63. This does the job for me.

Maybe the library should indeed be split up - but it should still only be included if a datepicker is actually used, not always if the jquery_ui_datepicker is enabled.