Remove sample date from date field error message and title attribute

If I'm understanding, this issue is about the discrepancy between the html5 input, and the old description of the accepted format?

That does indeed look like a documentation/ux issue.

Help text is wrong on "Authored on".
submit patch change in help text

1. +++ b/core/lib/Drupal/Core/Datetime/Plugin/Field/FieldWidget/TimestampDatetimeWidget.php
   @@ -35,7 +35,7 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen
   -    $element['value']['#description'] = $this->t('Format: %format. Leave blank to use the time of form submission.', array('%format' => Datetime::formatExample($date_format . ' ' . $time_format)));
   +    $element['value']['#description'] = $this->t('Format: %format. Leave blank to use the date and time of form submission.', array('%format' => Datetime::formatExample($date_format . ' ' . $time_format)));
    
   

   +++ b/core/modules/content_translation/src/ContentTranslationHandler.php
   @@ -453,7 +453,7 @@ public function entityFormAlter(array &$form, FormStateInterface $form_state, En
   -        '#description' => t('Format: %time. The date format is YYYY-MM-DD and %timezone is the time zone offset from UTC. Leave blank to use the time of form submission.', array('%time' => format_date(REQUEST_TIME, 'custom', 'Y-m-d H:i:s O'), '%timezone' => format_date(REQUEST_TIME, 'custom', 'O'))),
   +        '#description' => t('Format: %time. The date format is YYYY-MM-DD and %timezone is the time zone offset from UTC. Leave blank to use the date and time of form submission.', array('%time' => format_date(REQUEST_TIME, 'custom', 'Y-m-d H:i:s O'), '%timezone' => format_date(REQUEST_TIME, 'custom', 'O'))),
   

   These just change some language, not the format sample.

There are a few problems here the we need to balance.

1. Per https://www.w3.org/TR/html-markup/input.date.html, the values going into and coming out of <input type="date"> are RFC 3339 / ISO 8601.

2. When you use a <input type="date">, it is up to the browser to support it or not and how to style the inputs (not sure about definitive reference here, but http://stackoverflow.com/questions/7372038/is-there-any-way-to-change-in... is the closest that I know of).

3. Some browsers don't support these inputs, and treat them as textfields. We have a polyfill for date, and hopefully a polyfill for time will land soon. Both of those rely on JS. Non-JS will have to use the format we define.

4. Drupal on the PHP side doesn't know what a browser supports nor how a user's machine is configured (it, user settings may not follow the Drupal locale settings).

We can't do anything to influence the browser side (eg, Chrome). We may or may not have a localization problem with the polyfill(s) (haven't looked into this much).

I think my suggestion here is to hide the help text when on a supported browser (we have Modernizr in core, but don't add the body classes for a CSS-only solution) or when using the polyfills.

Other thoughts?

On further testing with other browsers - looks like only chrome of the one's I've tested changes the placeholder input format.

Odd.

#11 Chrome and Opera are using the native HTML5 date element. Safari and Firefox are using the polyfill (the jQuery UI datepicker). With the polyfill, the entry is essentially a text input and the UI is a "helper" for filling it out.

Here is a patch that just removes the troublesome text. I also feel the "Leave blank to use the time of form submission." doesn't make sense, since it's populated when first hitting the node/add/ page and it doesn't update upon submission. Also it looks like the text differs in the content_translation module.

Thanks for working on an old issue.

1. +++ b/core/lib/Drupal/Core/Datetime/Plugin/Field/FieldWidget/TimestampDatetimeWidget.php
   @@ -35,7 +35,6 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen
   -    $element['value']['#description'] = $this->t('Format: %format. Leave blank to use the time of form submission.', ['%format' => Datetime::formatExample($date_format . ' ' . $time_format)]);
   

   Even if the text is bad, we need something there. We can't just remove the form element description all together.

2. +++ b/core/modules/content_translation/src/ContentTranslationHandler.php
   @@ -463,15 +463,6 @@ public function entityFormAlter(array &$form, FormStateInterface $form_state, En
   -      $date = $new_translation ? REQUEST_TIME : $metadata->getCreatedTime();
   -      $form['content_translation']['created'] = [
   -        '#type' => 'textfield',
   -        '#title' => t('Authored on'),
   -        '#maxlength' => 25,
   -        '#description' => t('Format: %time. The date format is YYYY-MM-DD and %timezone is the time zone offset from UTC. Leave blank to use the time of form submission.', ['%time' => format_date(REQUEST_TIME, 'custom', 'Y-m-d H:i:s O'), '%timezone' => format_date(REQUEST_TIME, 'custom', 'O')]),
   -        '#default_value' => $new_translation || !$date ? '' : format_date($date, 'custom', 'Y-m-d H:i:s O'),
   -      ];
   -
          if (isset($language_widget)) {
   

   Same note here about the #description, but we also can't remove the form element all together.

In this patch, if the parent entity is new, it will leave the field blank, and then indeed it will pick up the date of submission from ::massageFormValues(). This makes the description text more accurate. I have removed the formatting suggestion, since it's obvious from the form element.

Also, I just leave the content_translation module alone. It's using a textfield with a date formatted "Y-m-d H:i:s O" instead of a timestamp, so for now, I'll just leave that form alone.

+++ b/core/lib/Drupal/Core/Datetime/Plugin/Field/FieldWidget/TimestampDatetimeWidget.php
@@ -29,13 +29,15 @@ class TimestampDatetimeWidget extends WidgetBase {
-    $element['value']['#description'] = $this->t('Format: %format. Leave blank to use the time of form submission.', ['%format' => Datetime::formatExample($date_format . ' ' . $time_format)]);
+    $element['value']['#description'] = $this->t('Leave blank to use the time of form submission.');

I don't think we can remove the format for the reasons expressed in #5. Rather something as suggested there will probably make more sense here:

I think my suggestion here is to hide the help text when on a supported browser (we have Modernizr in core, but don't add the body classes for a CSS-only solution) or when using the polyfills.

Attached is a quick patch against 8.5.x based on the suggestion by mpdonadio in #5.

I believe a javascript only approach should remove the formatting info from the description, but leave the "Leave blank to use the time of form submission."

Also, it would need to address the error message when someone manages to submit invalid data, and remove the formatting info from the error messages as well.

Here's an update to the non-javascript solution, that updates the error message to remove the formatting info, as well as the content translation description.

I think an additional thing that needs to be looked at is that the content translation "Authored On" element should copy as much as possible from the actual element, as it's current set to to a textfield and the default value is generated differently that the "Authored On" field on untranslated nodes.

I believe a javascript only approach should remove the formatting info from the description, but leave the "Leave blank to use the time of form submission."

I would have implemented that, but you said in comment #14, that you thought it didn't make sense.

Also, it would need to address the error message when someone manages to submit invalid data, and remove the formatting info from the error messages as well.

Yes, that's true, I didn't think of that. Thanks for pointing that out.

I was under the impression that it we came to the conclusion previously, that we can't just remove the formatting from the description in code and a JS-approach was needed. Since you continue to work in that direction, I wont update my patch for now. If we agree, that a JS-approach is what we want here, I can update my patch to address your comments. Just let me know.

Note that the current patch in #23 doesn't take the tooltip into account Date (YYYY/mm/dd).

After some consideration a JavaScript approach seems the only valid path forward here:

1. The Drupal backend has it's own format to validate.
2. The Drupal backend is agnostic to the frontend locale or browser settings (e.g. HTML5 support or not) which may have its own format.
3. The backend validation WITH the backend format is needed for a good UX for users that do not have JavaScript or HTML5 support. A message "The date is invalid" is not an option for these users.

To conclude, we must keep the backend message with the expected backend format.
As a consequence, it is up to the frontend to decide if it shows a custom widget (Jquery or HTML5 element) and to do the proper client side validation.

Adding a jQuery Timepicker for the Time element of the datetime field would also be a UX improvement for the datetime field when an HTML5 fallback is used.

Improving on #23 by removing formats in tooltips for date and time fields in DateTime element.

Additionally, no more usages of the formatExample helper in core so added deprecation notice.

As per https://caniuse.com/#feat=input-datetime, it seems like all browsers now support datetime input field but we need an usability review here first.

jibran's comment in #30 is incorrect. When Safari 12, the default browser in all currently shipping macOS devices, does not support the time input field, you cannot say that "all browsers" support the date and time input types. Even though Safari 12's usage is lower than IE11's (another problematic browser that does not support these input types), it cannot be discounted.

This new patch base on patch #21 (JS-only Patch) for Drupal 8.6.x.

Use correct filename for patchfile from comment #33 (Drupal 8.6.7).

+++ b/core/misc/date.es6.js
@@ -19,15 +19,25 @@
+          if (description_id) {
+            $context.find('#' + description_id).hide();
+          }

Hmmm. I wonder if this will have screenreader problems? IOW, do most current ones honor `display: none` and not read them?

Reroll of #29.

If I follow this correctly, this needs both the JS patch and PHP patch combined into a single patch file. Or if I'm wrong, please update the issue summary.

I think the JS patch could also use more code comments, specifically about why things are being hidden/shown.

I just change the time tip message with AM/PM, I know this is not good, but matching multiple browsers is out of my knowledge.

Wrong patch

Hi guaneagler,
Thanks for your interest but I think you have mis-understood the use of this issue queue. You can't just change the title, regress the core version, remove the associated issues & tags and upload a completely different patch. You are welcome to start a new issue with your hard-coded format change but this is not what the original issue is addressing.

Hence I have restored the previous title, links and tags.

Jonathan

@mpdonadio indicated at #3072906-57: Deprecate and remove jQuery UI datepicker that this issue may be obsolete if #3072906: Deprecate and remove jQuery UI datepicker lands in its current form. I just wanted to make people who participated in this issue aware of that other issue 😊

#44 The jQuery UI datepicker was applied for browsers that do not have a native datepicker. This issue is about the native datepicker, so #3072906: Deprecate and remove jQuery UI datepicker did not affect this.

Closed #2350025: Node authoring information description format inconsistency as a duplicate, adding credit.

Closed #2646454: Change the date format for the form display as a duplicate. Have not looked at credit because I am getting a headache.

Re-roll of #38, applies to 9.3.x and 9.4.x cleanly

Last patch forgot to remove one usage of createFormatExample, causing fatal errors on node edit. This patch fixes it.

Updated the @deprecated tag in the above patch, thanks!

The issue summary doesn't match the recent patches.

#58 works fine

Hi @jannakha, thanks for the feedback. However the tests on #58 did not complete because of two coding standards, so we cant set this to RTBC until at minimum we get green test passes.

New patch #63 for review. I added the @see link to this issue, but ultimately it should point to the Change Record if/when that is created.

Hi @jonathan1055

thanks for the fix! all good!

All tests passed on 4 April, but on 6 April we get a failure in modules/media/tests/src/FunctionalJavascript/CKEditorIntegrationTest.
They passed on 4 April

14  Media.Drupal\Tests\media\FunctionalJavascript\CKEditorIntegrationTest
✓  - testOnlyDrupalMediaTagProcessed
✓  - testErrorMessages
✓  - testPreviewUsesDefaultThemeAndIsClientCacheable
✓  - testEditableCaption
✓  - testDialogAccess
✓  - testAlt
✓  - testTranslationAlt
✓  - testLinkability with data set "linkability when `drupalimage` is enabled"
✓  - testLinkability with data set "linkability when `drupalimage` is disabled"
✓  - testEmbedPreviewAccess with data set "media_embed filter enabled"
✓  - testEmbedPreviewAccess with data set "media_embed filter disabled"
✓  - testEmbedPreviewAccess with data set "media_embed filter enabled, user not allowed to use text format"
✓  - testAlignment
✓  - testViewMode

This is not a file that we have altered in this patch. Has anything changed in core to affect this?

looks like it's a separate issue https://www.drupal.org/project/drupal/issues/3273626

Thanks @jannakha for finding #3273626: Resolve issue with tests for Drupal Media JavaScript causing database locks on SQLite.
Setting back to Needs Review. I can see we have another test run which passed.

It's not clear if this issue helps with setting the date format on the form as per #2936268: Date picker only works with US date and time formats. Can someone unpack that? Looks like Needs issue summary update is already on so not adding it.

@colan this is not a date format issue, it's just message issue. Date format is more of an issue of browsers and browser-native implementation of date-time field.

The last two test runs have ended with "build successful" which is actually an error, because not all the tests passed. It seems that some javascript tests failed:

TEST FAILURE: 2 errors during execution; 17 assertions failed, 1348 passed (6m 9s)
✖ Tests/toolbarTest
 – Change tab (5.661s)
   Timed out while waiting for element <#toolbar-item-user-tray> to be present for 5000 milliseconds. - expected "found" but got: "not found" (5166ms)

Maybe this is an unrelated test problem?

+++ b/core/lib/Drupal/Core/Datetime/Element/Datetime.php
@@ -390,6 +387,11 @@ public static function validateDatetime(&$element, FormStateInterface $form_stat
+   *
+   * @deprecated in drupal:9.4.0 and is removed from drupal:10.0.0. HTML date
+   *   input format should not be exposed to users, as browser widgets expose
+   *   input differently, varying by vendor, locale, device type, etc.
+   * @see https://www.drupal.org/project/drupal/issues/2791693
    */
   public static function formatExample($format) {
     if (!static::$dateExample) {

This should also have a @trigger_error(). Don't think we need a deprecation test since it's going to be a straight removal with no bc layer.

Since 9.4.0 is already released, this either needs to be deprecated in 9.5 (which we're generally trying to avoid) or in 10.1. I think given this is a bugfix and it's unlikely that contrib code is calling the method, in 9.5 for removal in 11.x might be best?

The issue is still tagged for usability review, has this happened?

Hi, on Drupal 9.4.8 the Description is now gone and you can use the help text if you want to customise it. There is a ticket to review date related elements for UX best practices. https://www.drupal.org/project/drupal/issues/2897397 For now there are no labels so this issue is fixed in that respect.

All that remains is making sure we haven't missed anything

Actioned #74 and rerolled against 10.1.x. The issue summary looks good to me as well.

As mentioned in #76, the description mentioned in the ticket's original problem statement has been removed from Drupal version 9.4.8 and above. So need confirmation whether we really need a re-roll here for Drupal 10 version.

Re #76 and #79 - while the description for the created date has indeed been changed, this ticket is still removing it from other areas - the input's title and error message.

Updated patch with stan baseline fix.

Screenshots:

Before:

After:

@acbramley: I am a bit confused. Does this issue require more work or is it RTBC?

@xmacinfo no, it needs reviewing. It was set to Needs Work in #74 based on the deprecation error. That has been fixed. However, it's also tagged for usability and accessibility review, however I'm not 100% sure if that is required given it's a fairly straight forward change.

@acbramley: Thanks for the clarification. Let’s see if we can have usability and accessibility reviews.

Usability review

We reviewed this issue at #3347230: Drupal Usability Meeting 2023-03-17.

That issue will have a link to the recording, for the record the attendees were: myself, @rkoller, @simohell, @shaal and @blackbamboo.

After discussing various options, the group made the following recommendations:

1. We recommend [field_name] value is incorrect. Enter a valid date and time. for the error message text. For example: "Authored on value is incorrect. Enter a valid date and time".
2. We also recommend completely removing the title attributes.
3. In a follow-up issue, we should explore making the validation specific to the individual date or time elements, right now if the error is triggered by the time element, both the date and time elements are highlighted in red. This is not ideal, and we should find a way to highlight the specific element that is causing the error.
4. We used the "Authored on" Node field to test with, and we noted that the description of that filed could be improved, it simply says "The time that the node was created.", we felt this could be improved by saying "date and time", we also try to avoid the term "node" in the user interface and instead prefer "content". Obviously that is out of scope for this issue, but as with point three it would be useful if a follow-up issue could be filed for that.

Adding the needs followup tag for points 3 and 4.

Thanks @AaronMcHale!

Re 1. - This actually already is the error message (for the Authored On field at least). For other date time fields added to the content type there is no #title attribute on the $element since that comes from the fieldset itself. There is no other way I can see to get the title for those fields, this should be investigated in a follow up imo as it could have large flow on affects for getting that info in there.

The error messages in #80 were when using one of those custom fields (i.e a Date time field added to the Article content type). When testing the Authored on field, it shows correctly:

The problem with the title is already a bug in the current error message as well. Since this issue is just about removing the format I think that should be moved to a follow up.

I've removed the title attributes per #84.2.

So looks like we need 3 follow ups?

1. Improve Datetime validation to be specific to Date or Time input
2. Improve Authored On description (fwiw this is manageable using base field overrides although most people wouldn't be interacting with that system.
3. Add field title to Datetime validation error for configurable datetime fields.

Back to NR

@acbramley Thanks for the reply.

The text that is being proposed is [field_name] value is incorrect. Enter a valid date and time., which is different from what is in the patch in this issue. Note that we have specifically removed the word "invalid".

We probably then should have a follow-up issue to, as you say, remove the title attribute on the fieldset.

So it would be Authored on value is incorrect. Enter a valid date and time? Honestly that reads much worse than what's there currently to me.

To be clear - it's not a title attribute on the fieldset. When a non-base date field is rendered on the form it is wrapped in a fieldset. That fieldset contains the field's label in its legend, it is not a label on the element passed in (i.e not in $element['#title']. Therefore it is not possible to include that label in the error message (that I can see)

Not 100% sure what needs accessibility review. If general direction can be pointed out I'm not the worst 508 tester.
Moving to NW for all the follow up tickets being discussed.

Because of the recent changes in 10.1.x, the patch in #85 does not get applied. So just re-rolling it to be compatible with 10.1.x

please review.

please review.fixed CCF.

+++ b/core/lib/Drupal/Core/Datetime/Element/Datetime.php
@@ -374,7 +369,7 @@ public static function validateDatetime(&$element, FormStateInterface $form_stat
+          $form_state->setError($element, t('The %field date is invalid. Please enter a date in the valid format.', ['%field' => $title]));

Why has this changed from "in the correct format" to "in the valid format"? Please also post interdiffs with your changes and reasoning behind them.

I've fixed an issue in tests. Here is a patch to review.

Fixed small issue in the tests after deeper check

D11 patch where Datetime::formatExample() was completely removed

"Please" is a forbidden word, so here is a new patch for D11

Ok, need to upload test only patch + a full patch

Ah, there was issue in the test, I did some cleanup, should be good now

Whoop whoop, now it's time for D11 attempt

IS mentions Deprecate DateTime::formatExample but didn't see in the patch

Also was previously tagged for follow ups have those been opened?

Hi @smustgrave,

Patch #105 for drupal 10 has a deprecation.
Patch #107 for drupal 11 does not have deprecation because the method was completely removed.

Am I missing something?

11.x is the current development branch that D10 releases will be cut from. So the deprecation needs to be included.

@smustgrave thanks, I missed that momoment, here is updated patch.

Looks good to me

There were a number of follow ups being discussed in 84-88 have those been opened?

Follow ups created

#3382734: [PP1] Improve Datetime validation to be specific to Date or Time input
#3382737: Improve Authored On description
#3382738: Add field title to Datetime validation error for configurable datetime fields

Needs accessibility review was added a long time ago (talked about PHP and JS changes which doesn't apply anymore), I don't feel like this new solution needs that anymore. It's also been through a usability review already.

Thanks @acbramley for cleaning those tags up!

Everything looks good but can deprecation be updated for 10.2 vs 10.1 please.

Then think it's good to RTBC!

Please review.

I'm triaging RTBC issues. I read the IS and the comments. I didn't find any unanswered questions or other work to do.

Thanks for making this review easy by having an up to date IS and screenshots.

Thanks for making the followup issues, #115. I think should be siblings of this one so they are easy to find and to consider the 'big picture' That is, they should have the same parent as this one.

I read the patch, not a code review.

1. +++ b/core/lib/Drupal/Core/Datetime/Element/Datetime.php
   @@ -5,8 +5,8 @@
   +use Drupal\Core\Form\FormStateInterface;
   

   The re-sorting is out of scope.

2. +++ b/core/lib/Drupal/Core/Datetime/Element/Datetime.php
   @@ -389,8 +384,14 @@ public static function validateDatetime(&$element, FormStateInterface $form_stat
   +   * @deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. HTML date
   +   *   input format should not be exposed to users, as browser widgets expose
   +   *   input differently, varying by vendor, locale, device type, etc.
   

   By practice, this is the same deprecation message as in the trigger_error. So, lets keep them the same.

   @deprecated in drupal:10.2.0 and is removed from drupal:11.0.0. There is no replacement.

3. +++ b/core/lib/Drupal/Core/Datetime/Element/Datetime.php
   @@ -389,8 +384,14 @@ public static function validateDatetime(&$element, FormStateInterface $form_stat
   +   * @see https://www.drupal.org/project/drupal/issues/2791693
   

   There should be a blank line before the @see.

4. +++ b/core/lib/Drupal/Core/Datetime/Element/Datetime.php
   @@ -389,8 +384,14 @@ public static function validateDatetime(&$element, FormStateInterface $form_stat
   +   * @deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. HTML date
   ...
   +    @trigger_error(__METHOD__ . ' is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0. No replacement provided. https://www.drupal.org/project/drupal/issues/2791693', E_USER_DEPRECATED);
   

   This is close, but does not meet coding standards, it needs 'See'. And the version needs to be updated. Also changing the middle sentence to be consistent.

   is deprecated in drupal:10.2.0 and is removed from drupal:11.0.0. There is no replacement. See https://www.drupal.org/project/drupal/issues/2791693

   See @trigger_error() format

@rpayanm, this issue has been using a patch work flow for many years and can continue to do so. There is no need to create an MR.

Finally, removing tags per the guidelines and setting back to NW.

Edit: fix formatting

Addresses #123

Added CR and updated messages.

@acbramley, thank you for making the changes.

I have reviewed the changes and read the CR and setting back to RTBC.

Adding some issue credit.

Committed 55efc8e and pushed to 11.x. Thanks!

Did my best with issue credit but this is a very long issue so apologies if any omissions.

Published the CR

Just rerolled a patch for 9.5.11, without tests

Patch for 10.1.x branch to fix the issue.

@HitchShock @kavbiswa I don't think there is a plan to port this change to 10.x and 9.x? But I could be wrong.

Hi @jonathan1055
Sure, the solution will be released in the last Drupal version. We added these patches only for those who want to apply it to the older versions

Can someone please reroll the patch for Drupal 10.2.0?

@RichardDavies no need, the patch was committed to 11.x before 10.2.0 was released and is therefore available in 10.2.0 :)

@acbramley Ah, thank you for the clarification. I didn't understand how the 11.x branch worked and couldn't find this item listed in any of the 10.2x release notes so I assumed it wasn't in the 10.2 release. But I've just confirmed it is indeed there.

Thanks @RichardDavies for confirming that the fix is in 10.2. I also did not know that the commit to 11.x would be silently ported to 10.x without some commit message being shown here. I'm sure many people knew this, but I didn't, and I always look for commit messages in the comments of the issues. Maybe I should also remember to search in the commit history of the branch.

Viewing https://git.drupalcode.org/project/drupal/-/commits/10.2.x and scrolling down to the right dates we see

The commit id 5404ebfb is exactly the same as the 11.x commit so I am still unsure whether this was a cherry-pick or whether the 11.x branch was actually the 'same' as the 10.2.x branch back then.

It is the new Drupal core branching:
https://www.drupal.org/about/core/blog/new-drupal-core-branching-scheme-...

Thank you @Martijn de Wit, I knew there was something big going with branch naming, and that post explains it all.

I believe an unintended change was introduced with this issue. If a timestamp field provided a default value or default value callback, that field-level default value is no longer used as the default value in the entity form when creating a new entity. There is a check in TimestampDatetimeWidget::formElement() to only use the field item delta value if the entity is "not new": https://git.drupalcode.org/project/drupal/-/commit/5404ebfb11155b53dee0e...

A timestamp field with a default value might be a common use-case when the field is required (the user must provide a value) but the application would like to preset a default value to the current time for convenience. For an example see #3396419: Make timestamp required

@paul121 nice find, I will try and track down in this issue why that change was made.

Are you able to create a new issue with your bug report and steps to reproduce from a fresh Drupal install?

Originally added in #16 I'm not quite sure why, as you say default values are generally used on new entities so it doesn't make much sense to me. Unfortunate that we didn't pick it up sooner!

Thanks for the *fast* response @acbramley! I've opened a new issue #3414883: [regression] datetime_timestamp widget does not use default field value