- Status changed to Needs review
almost 2 years ago 2:26pm 16 February 2023 - ๐บ๐ธUnited States rainbreaw
During the A11y office hours, we added an AI generated summary to the description. This still needs to be validated by a human, but we are hoping this might help us make it easier to address issues.
- Status changed to RTBC
over 1 year ago 8:44pm 22 April 2023 - ๐ง๐ทBrazil Diego_Mow
Patch 92 worked in both 9.5 and 10.1.
I'm moving it to RTBC.About AI generated summary, I reviewed it and looks pretty similar to initial summary.
It looks fine for me. - last update
over 1 year ago 28,652 pass, 4 fail - ๐ฌ๐งUnited Kingdom longwave UK
As far as the API goes here I would like to repeat my comment from #67
We don't have many "disable" options in Drupal; instead should we name the flag
#inline_form_errors_summary
and default it to TRUE, and then allow FALSE to disable it?Having said that we also have
#error_no_message
three lines above, so should we be consistent with that somehow? - Status changed to Needs work
over 1 year ago 11:19am 24 April 2023 - ๐ฌ๐งUnited Kingdom alexpott ๐ช๐บ๐
I agree with @longwave. We discussed how that would be implemented. I think we need to implement hook_element_info_alter and add
#inline_form_errors_summary
there with a default of TRUE. I think the implementation would be:function inline_form_errors_element_info_alter(&$info) { $info['form']['#inline_form_errors_summary'] = TRUE; }
- ๐ฌ๐งUnited Kingdom alexpott ๐ช๐บ๐
Also I'm really confused about why and why you'd use the setting here #disable_inline_form_errors_summary vs #disable_inline_form_errors vs #error_no_message. There's way too many double negatives here and also with the current implementation #disable_inline_form_errors_summary has to be set on the form level - but the comments and issue summary here indicate that there are still situations when it will be shown even if this is set...
We need to add docs of these three variables and where and how they can be used. And perhaps when we write better docs a good name for the new thing will be more obvious.
- Status changed to Needs review
over 1 year ago 11:37am 24 April 2023 - last update
over 1 year ago 29,297 pass, 4 fail - ๐ง๐ทBrazil Diego_Mow
Patch #100 goes with followed suggestion: use key #inline_form_errors_summary
Note: we also have #disable_inline_form_errors, so consistency about naming here is really broken.
My honest suggestion is that we have this issue solved since this feature is really overtime and open a task to discuss only consistency of naming for those keys. - ๐ฌ๐งUnited Kingdom alexpott ๐ช๐บ๐
+++ b/core/modules/inline_form_errors/tests/src/Unit/FormErrorHandlerTest.php @@ -177,7 +177,7 @@ public function testErrorMessagesNotInline() { - $this->testForm['#disable_inline_form_errors'] = TRUE; + $this->testForm['#inline_form_errors_summary'] = FALSE;
Not sure why this is changing - looks like we'd be losing coverage.
The last submitted patch, 100: 2880011-100.patch, failed testing. View results โ
- Status changed to Needs work
over 1 year ago 11:16pm 24 April 2023 - ๐ง๐ทBrazil Diego_Mow
Good cache alexpott. This was mistakenly changed.
Adding patch 103 without this change.
- last update
over 1 year ago 29,297 pass, 4 fail - ๐ฌ๐ทGreece fotisp
Path #103 disables summary errors on all forms.
+ // Do not show links to elements when the errors summary is disabled. + elseif ($is_visible_element && $has_id && $form['#inline_form_errors_summary']) { + unset($errors[$name]); + }
&& $form['#inline_form_errors_summary']
should be
&& !$form['#inline_form_errors_summary'] - ๐ฎ๐ณIndia _utsavsharma
Tried to address the pointer in #105.
Please review. - last update
over 1 year ago 29,463 pass, 2 fail - Status changed to Needs review
over 1 year ago 8:30am 22 August 2023 - last update
over 1 year ago 29,470 pass - ๐ฎ๐ณIndia vsujeetkumar Delhi
Fixed the fail tests for 10.1.x, Please have a look.
- Status changed to Needs work
over 1 year ago 10:49am 26 August 2023 The Needs Review Queue Bot โ tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide โ to find step-by-step guides for working with issues.
- ๐ช๐ฌEgypt suzy.william
suzy.william โ made their first commit to this issueโs fork.
- First commit to issue fork.
- Merge request !9079Issue #2880011 by kriboogh: Add a #disable_inline_form_errors_summary property... โ (Closed) created by kriboogh
- ๐ง๐ชBelgium kriboogh
Created a MR from patch #107.
Added patch for composer use.
Applies to 11.x and 10.3.x.Still some weird error is happening with the use of "withConsecutive" (which the MR solves, but phpstan still complains about...)
- ๐ง๐ชBelgium kriboogh
kriboogh โ changed the visibility of the branch 2880011-add-a-disableinlineformerrorssummary to hidden.
- Merge request !9083Issue #2880011 by kriboogh: Add a #disable_inline_form_errors_summary property... โ (Open) created by kriboogh
- ๐ง๐ชBelgium kriboogh
Corrected MR from latest 11.x.
Added patch for composer.
Applies to 11.x and 10.3.x.