Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
With Inline Form Errors module enabled errors on datetime render (form) elements are repeated.
The error should just be thrown once.
Or if there is an specific error for one of the input/selects it should be shown only for that part of the element.
Before:
Comment | File | Size | Author |
---|---|---|---|
#46 | 2827034-nr-bot.txt | 5.79 KB | needs-review-queue-bot |
#38 | Screenshot_20200414_122224.png | 13.72 KB | johnchque |
#36 | Screen Shot 2020-01-30 at 1.48.59 PM.png | 113.39 KB | mpdonadio |
#35 | 2827034-35.patch | 15.7 KB | mpdonadio |
#35 | 2827034-35-TEST-ONLY.patch | 12.11 KB | mpdonadio |
Comments
Comment #2
dmsmidtI've quickly looked through core and it looks like this element is hardly or even never used.
And furthermore the element can't be chosen via the admin interface.
Comment #4
NitebreedThis element can be added with the Webform module via the admin interface. Therefore, I'm resetting the priority.
Comment #5
dmsmidtComment #6
ericmulder1980 CreditAttribution: ericmulder1980 as a volunteer commentedWorking on this at the a11y-sprint-2017
Comment #7
Krilo_89 CreditAttribution: Krilo_89 at Synetic commentedAlso worked on this issue at the a11y-sprint-2017. (@ericmulder1980 knows about this)
Changing:
'#error_no_message' => FALSE,
to:
'#error_no_message' => TRUE,
results in 1 error message on the parent element and no error messages on child elements. In that way the Datelist elements render without multiple repeated errors between the elements. However, to due the way error messages bubble up the possible individual errors per date part are not visible anymore.
So next to showing only a single error message for the Datelist field we also have to let the user know which part (or parts) is/are problematic.
That's why the message is changed to the one below, where the errors for the child elements were added:
$form_state->setError($element, t('The %field date is incomplete. A value must be selected for %parts.', ['%field' => $title, '%part' => implode(', ', $all_empty)]));
Comment #8
Krilo_89 CreditAttribution: Krilo_89 at Synetic commentedAdded related issue. #2486019: Wrong validation messages in Datelist::validateDatelist()
Comment #11
BerdirThere is a similar problem for date range fields with date form elements, for example when entering an end date that is before the start date.
Separate issue I guess?
Comment #13
mpdonadioWe need a test to demonstrate this and prevent regressions.
Given the size of the current functional datetime/daterange field tests, and since IFE isn't in the minimal or standard profiles, I would prefer a standalone test that enables IFE and tests the bug.
I wouldn't object to merging this with #2927452: Date range fields have repeating validation messages with IFE.
Comment #14
johnchqueWill work on this.
Comment #15
johnchqueAdding the patch of #2927452: Date range fields have repeating validation messages with IFE. Had some problems with my local env.
I've noticed something here:
With IFE, the field is refered as Year only, not to every field. Will investigate.
I have closed the other issue as duplicated so we can continue in this one only.
Comment #17
johnchqueFIxing tests, adding specific tests for IFE, added title to the field so the problem described in #15 is fixed.
Comment #18
BerdirI don't think the title should be visible like that. Is there any way to achieve that without making it visible? I guess we could use #label_display hidden?
Comment #19
mgifford@Bedir do you want it to just say "A value must be selected for year, hour minute, ampms"?
Given that it is right next to the label & input I would be fine with that.
Comment #20
johnchque@mgifford, @Berdir refers to the title "Test (value 1)" and "Test (value 2)" that were not present in the screenshot that I uploaded in #15.
Titles in bold where added with the last change I made:
To use the correct title in the top message that inline form errors displays. I tried following the suggestion of @Berdir but it seems that the title is still being displayed.
Comment #21
mgiffordThanks for the clarification @yongt9412.
Comment #22
BerdirI think we should revert that and open a follow-up to improve the label.
The problem is that this is not a standard form element where #title_display works, it is a custom template datetime wrapper.
At the same time, the title is detected in \Drupal\Core\Form\FormElementHelper::getElementTitle().
I only see two ways out, either we add something to respect #title_display or something similar to those preprocess/template functions (I think we could add it to preprocess and it should be respected automatically be all templates unless they do something crazy) or we add a special property that we can check in that method.
But I think we can deal with that in a separate issue and focus on fixing the duplicates here.
Comment #23
johnchqueSo reverting the title addition.
Opening follow-up: #2958626: Display the field label in IFE messages with datelists
Comment #27
Luke.LeberReroll for 8.9 with a small change fixing a typo.
Comment #28
Luke.LeberComment #29
jhedstromThe tests are failing due to some deprecation notices, so the test will need updating to address those.
Comment #30
BerdirAdded the $defaultTheme property.
Comment #32
Luke.LeberComment #33
Luke.LeberComment #34
Luke.LeberComment #35
mpdonadioRe-running #32 and just the new test. Also doing a manual test.
Comment #36
mpdonadioPatch doesn't work for Datetime Range fields configured for the Datelist form entry:
Comment #37
johnchqueGonna review this.
Comment #38
johnchqueOK, so it seems that the message not being displayed is somehow different.
The code in Drupal\datetime_range\Plugin\Field\FieldWidget\DateRangeWidgetBase adds an error message that checks whether the end date is before the start date and throws an error if that is the case. The tests added here for date range are passing as we are testing that message.
However, that file DateRangeWidgetBase extends from DateTimeWidgetBase which uses a type=>datetime field. This field has its own validation but it is not displayed in the DateRange field. Not sure if that is part of the scope of this issue as we just want not to have duplicated error messages in these fields, which is the case.
I believe that opening a follow-up to address that problem would be best. Thoughts?
This is how the error message is displayed.
Comment #39
johnchqueComment #46
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #47
mgiffordI think this would confuse the user so listing it as https://www.w3.org/WAI/WCAG21/Understanding/error-identification.html