This planning issue shows all followups from #1493324: Inline form errors for accessibility and UX.

Inline Form Errors is currently Alpha.

Note that Inline Form Errors was under consideration for removal from core based on limited progress on these blockers, in accordance with the experimental module policy. The following "must-have" issues needed to be addressed (fixed or mostly fixed) by 8.3.0's beta phase in order for Inline Form errors to remain in core.

*) These issues are problematic even without IFE!

Must have

Complete by February 1, 2017

Complete by March 13, 2017

Complete before module is marked RC

Should have

Complete by February 1, 2017

Complete before module is marked beta

Complete before module is marked RC

Could have (non-blocking bugs and normal followup work)

Minor issues (Non blockers / Nice to haves)

Testing and reviewing results

To get a quick overview of what has been implemented, how IFE looks, and to test open issues you can use this helper module:

https://github.com/dmsmidt/errorstyle (Credits: vijaycs85, dmsmidt, skaught-, mparker17).

Browse to http://your-test-site.localhost/error-style/form

Note: HTML5 errors are disabled on the test form.

Click on this image for an overview off all form elements. Note that so many errors on the same page is unrealistic and that the image is not exhaustive in terms of user interaction with form elements.

Files: 
CommentFileSizeAuthor
#58 ife_overview_thumb.png14.59 KBdmsmidt
#58 ife_overview.png469.19 KBdmsmidt
#5 checkboxes.png112.97 KBTR

Comments

tim.plunkett’s picture

Category: Task » Plan
mgifford’s picture

Issue summary: View changes
alexpott’s picture

Issue summary: View changes

We don't have to care about stark - see #2349711: Remove all visual from stark

dmsmidt’s picture

Issue summary: View changes

Plan summary changes to address #1493324-578: Inline form errors for accessibility and UX and #1493324-588: Inline form errors for accessibility and UX
- Split out the visual inconsistencie issue's as requested by Bohjan.
- Combined and rewrote the issues for Ajaxified fields, they can't be handled without the other.

TR’s picture

FileSize
112.97 KB

Go to /admin/modules/uninstall
Submit the form without checking any checkboxes.
This is what you get:
module uninstall form

Neither the message nor the inline errors are very useful.

Before #1493324: Inline form errors for accessibility and UX it used to just show a message "No modules selected." A better message would be something like "You must select at least one module to uninstall".

But a message complaining about the first module that happens to be on this list, with inline errors for every single checkbox, is a big step backward in usability and accessibility for this page.

tim.plunkett’s picture

Both of these are in the issue summary.

Better style errors on radio / checkbox (groups).
Radios / Checkboxes focus styling wrong when marked as having an error

TR’s picture

No, as far as I can tell this wasn't raised in #1493324: Inline form errors for accessibility and UX and is different than the really vague bullet items in this issue summary which have no links or details indicating what they refer to. I *did* look through all 595 comments (and the accompanying screenshots) in that issue before I posted, so please take the time to look at what I wrote before dismissing it ...

What I show in #5 is *not* "Radios / Checkboxes focus styling wrong when marked as having an error", which was first raised/demonstrated in #435 and is clearly all about the focus, while focus plays no part in what I show in #5.

And it is *not* "Better style errors on radio / checkbox (groups)". Although it is less clear what this refers to, it seems to be about the difference between the style and placement of the inline error shown on single radio/checkbox elements versus on grouped radios/checkboxes. This may also refer to the whole #error_use_parent/#error_no_message property discussion and the whole "not a form element" discussion, but that was claimed to have been addressed by the original issue (see #519), not left for later. Specifically, the screenshots in the issue summary for #1493324: Inline form errors for accessibility and UX and in the comments starting with comment #381 show one and only one inline error for a radio or checkbox group, whereas my screenshot shows an inline error for each and every checkbox.

"Better" implies the situation is OK now, but could be improved. That's certainly not the case - what would a screen reader do for the page in #5? As I said, it's a big regression for usability and accessibility on this page. This current issue is supposed to be about "inconsistencies and annoyances", and the two bullet points you listed are for "nice to have" minor problems, which does not describe what I posted.

Additionally there is no mention in #1493324: Inline form errors for accessibility and UX of the wrong message I show - "1 error has been found Uninstall block module" - the Block module was not checked for uninstall, and in fact can't be checked because of dependencies. The actual error has nothing to do with the Block module checkbox and the actual message doesn't say what the real problem is.

Perhaps the problem lies in the module uninstall form rather than the changes introduced in #1493324: Inline form errors for accessibility and UX. Regardless, it's a real problem. My only question would be whether this should be addressed in this "plan" or whether it should be a new major/critical release-blocking bug report.

mgifford’s picture

@TR I think it should be a new issue. Thanks for raising it.

tim.plunkett’s picture

@TR I'm sorry, I was not intending to dismiss your findings at all, just pointing out that we knew groups of checkboxes were not styled ideally. A dedicated issue for what you describe is very welcome.

This "plan" is just a meta, nothing actionable is being done here. Just a place to gather all of the issues.

xjm’s picture

Issue summary: View changes

So I just ran into #2346773: Details form element should open when there are errors on child elements on the node edit form. I disagree that it is "minor" or "nice to have" as it is actually quite a nasty usability regression.

The issue with the details element remaining collapsed exists in HEAD, but previously, at least the validation error message was visible so the user would hopefully figure out what was going on. Now the informative validation message is actually hidden, and furthermore it now comes with a link that seems not to work at all. I can imagine this bug being extremely frustrating for users and making it seem like Drupal is just broken. It's bad enough that I'd consider it borderline critical. I've bumped it to major for now since it is still technically possible to use the form when there is a validation error.

I'd suggest making that issue a top priority in this followup work.

TR’s picture

bojanz’s picture

The inline errors are horribly buggy :(

I've tracked down the cause of TR's #2509268: Inline errors repeated on child elements in module uninstall form, and opened another one: #2512306: Inline errors not shown for details elements.
The first one might not be a critical, but the second one smells like it.

tim.plunkett’s picture

Unfortunately, I think we need to give up on this effort for 8.0.x, and try again for 8.y.x.
#2578561: Move "Inline Form Errors" functionality to optional module and restore D7-style form errors by default

mgifford’s picture

Issue tags: +wcag

This is an important accessibility tool. Many government & educational sites have chosen Drupal for it's accessibility, and form errors are an important part of WCAG 2.0 AA. Taking out the inline form error code would be a big step back for accessibility.

Taking out this code, this late in the game may also cause other problems.

I don't know if any of the outstanding issues are going to be that hard to solve if we make them priorities.

tim.plunkett’s picture

Can you elaborate on "other" problems?
Ideally this can be solved from contrib until 8.1.x, and we can fix it there properly.
But it is *broken* in HEAD right now, we need to cut our losses.

mgifford’s picture

I don't know what other problems there might be.

I thought it was here but must have been another issue - #2192419: Use a WCAG-compliant fieldset (fieldgroup) for #type radios/checkboxes - In the past I know that we've been reluctant to remove code that has in Core for a while, and this patch has been in 4 months.

I don't know what overlapping code there might be and it's a complicated enough code base that a lot can change in 4 months. You're a damn solid programmer so you've probably got everything, but still.

The community focus has been on the critical issues for the last few months. Maybe now that most of those are out of the way it will be easier to take on some of these other ones.

I'm eager to get D8 out. I just know what a massive job it has been to get this into Core. Over 600 comments in the main issue and several other issues with lots of traffic too.

rootwork’s picture

Version: 8.0.x-dev » 8.1.x-dev

So presumably this is 8.1 now. It looks like many of the child issues could use some love.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

lokapujya’s picture

Should #2346773: Details form element should open when there are errors on child elements be split for vertical tabs?

An existing issue marked as a duplicate already exists for the vertical tabs: #2531700: Fragment links to children elements in closed grouping elements don't work

abinayaa.k’s picture

Hi,
As per #1 content is saving Without date and time. Even how it can enable for after the content was published? purpose of removing date and time? Then We have mm/dd/yyyy format but error occurring format is 2016-03-05 like that.

mgifford’s picture

Issue summary: View changes
mgifford’s picture

Issue summary: View changes
mgifford’s picture

Issue summary: View changes
SKAUGHT’s picture

SKAUGHT’s picture

xjm’s picture

SKAUGHT’s picture

I guess the big question is: how stable is stable?

which issues are actually blocking a better dev tag for IFE

clip from 2702545#26 IMO, at this point the 'worst breaks' reported are

otherwise: https://www.drupal.org/node/2687249 -- 'AJAXified fields' will probably needs larger changes to the File API, as it still largely uses drupal_set_message, not triggering form errors, as it is.

#2457373: is more of a nice to have, rather than any kind of breaker.

andrewmacpherson’s picture

Component: forms system » inline_form_errors.module
SKAUGHT’s picture

follow up to: Links to a CKeditor don't work because the original field is hidden

this has somehow been addressed. shot from current 8.2.x dev line.

SKAUGHT’s picture

Issue summary: View changes

adding del tag to ckeditor point

lokapujya’s picture

https://www.drupal.org/node/2346773 - details related 'openness'

We don't know which array structure to target for errors. See comment #45 and the couple comments before it.

dmsmidt’s picture

@SKAUGHT #29

From the image I can't see that the link actually works. The problem was that clicking on (in case of your screenshot) 'body' wouldn't jump to the actual field.
So if the body field is not in view it should, like an anchor, scroll into view when the link in the error message is clicked. Can you confirm that?

SKAUGHT’s picture

Issue summary: View changes

sorry, i missed the intention in lue of the display of the error message. No, link does not jumpto hidden textarea.

would you happen to know of this as an actual issue id?

dmsmidt’s picture

dmsmidt’s picture

Issue summary: View changes
xjm’s picture

Title: Finalize new inline form errors » [meta] Roadmap for stabilizing Inline Form Errors module
Issue summary: View changes
SKAUGHT’s picture

@lokapujya [#31]
i would like to switch over to help you on that issue. however, i may not be able to for a few weeks as i'm about to move and am going to be without a connection shortly enough as it is.

SKAUGHT’s picture

i've completed a work-around patch for https://www.drupal.org/node/2509268. alhtough this doesn't address #tree element issue (which is the true problem behind that ticket) is does address the multiple errors.

SKAUGHT’s picture

2560467#5 | Inline errors not shown for container elements
IMO: i don't think that container elements should be able have form errors triggered against them. it's an odd 'want to have' as form dev's should be triggering errors to an actual form item

it should only be checkboxES and radioS [plural] where the error should trigger of that whole group.

SKAUGHT’s picture

Issue summary: View changes
Wim Leers’s picture

dmsmidt’s picture

Issue summary: View changes
SKAUGHT’s picture

Issue summary: View changes

removing #2514730 from sub-line. has been marked as duplicate.

dmsmidt’s picture

Issue tags: +DevDaysMilan
dmsmidt’s picture

Issue summary: View changes
dmsmidt’s picture

Issue summary: View changes
dmsmidt’s picture

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dmsmidt’s picture

Issue summary: View changes
xjm’s picture

Issue summary: View changes

Prior to the release of the 8.2.x beta, the core committers agreed to give IFE until 8.3.0 beta to complete the required work instead of 8.2.0, since the expectation for experimental modules was not yet clear as of 8.0.0.

If the module is removed from core, it is still eligible (at least potentially) to be added back once these issues are addressed.

Thanks everyone for ongoing contributions to this accessibility feature. With your help, there is still time to make it a stable part of 8.3.0.

mgifford’s picture

Thanks for the information & encouragement @xjm.

dmsmidt’s picture

Issue summary: View changes

Work has been done, yeah!
Updating the overview.

dmsmidt’s picture

Issue summary: View changes

Update issue summary to reflect which issues are Drupal generic issues and should be fixed regardless of IFE.

dmsmidt’s picture

Issue summary: View changes
dmsmidt’s picture

Issue summary: View changes
emarchak’s picture

@dmsmidt If they're Drupal generic issues, does that mean they're still blockers for IFE since they can be extracted out?

dmsmidt’s picture

Issue summary: View changes

@emarchak, thank you for that interesting question. It made me rethink about the priority of issues.
To get a better grip on thing I re-read: https://www.drupal.org/core/issue-priority

I decided to add the 'normal' priority to the list. Because in general IFE is mostly ready, and has imvho no serious regression in UX anymore. Just a couple of varying nuisances, nothing that renders the site unusable or completely not understandable (afaik).

In general, some of those issues are problematic in the current version of Drupal core even without IFE, but are even more problematic with IFE on. Because it removes all error messages at the top of the page and adds anchor/hash links to the actual errors, they become extra apparent and in those cases IFE generates non working Links. I kept those as Major (blockers).

In this new ordering I tried to have a balanced priority in the context of primary related to IFE and Drupal in general.
Please, please, review this with me and let's get consensus about this priorities. I think how we stand doesn't look too bad anymore.
All individual issues more or less already had these priority statuses, based on this reprioritization I'll make #2729663: Fragment link pointing to <textarea> should be redirected to CKEditor instance when CKEditor replaced that textarea major.

dmsmidt’s picture

Title: [meta] Roadmap for stabilizing Inline Form Errors module » [meta] Roadmap for stabilizing Inline Form Errors module (IFE)
Issue summary: View changes
FileSize
469.19 KB
14.59 KB

Add helper module info and overview picture.

marcvangend’s picture

Even though I'm aware of the policy for experimental modules, I'm surprised to see that inline form errors is at danger of being removed from core. This is a huge UX/accessibility improvement and a lot of work has gone into this. Awesome ideas and new initiatives have come up for 8.3, but we should also be able to finish what we started, right?

So how can we get this ready for 8.3?

First of all I think Daniel did a great job in triaging the remaining issues. There aren't that many blockers, and even those issues are in fact problems that already exist in core. I'd love to see if core maintainers agree with the list of blockers.

Second, I'd like us to consider if those 3 remaining blockers are in fact blockers. IMHO the module, even in its current state, solves much more problems than it creates and I do not hesitate to use it in client projects.

Finally, if we want to get some momentum behind this, we need to spread the word. For starters, I really think IFE belongs in the list of Ongoing Initiatives at https://www.drupal.org/core/roadmap#ongoing. Perhaps core office hours can help too.

dmsmidt’s picture

Issue summary: View changes

@marcvangend, thank you for this positive feedback.

(Added minor issue #2827034: DateList inline form errors are repeated to the list).

Wim Leers’s picture

Perhaps core office hours can help too.

+1. This makes it easier for others to help out. If you tweet about novice tasks, meetings, etc, I'll retweet it, and I'm sure many others will too.

xjm’s picture

Thanks for the updates on this issue. I'll try to review it soon, hopefully next week. Regarding the roadmap handbook page I think we meant to change that to point to the ideas queue anyway. I'll check with @webchick about plans for that page.

If they're Drupal generic issues, does that mean they're still blockers for IFE since they can be extracted out?

Yes, they are still blockers, because even if they exist in other code than IFE, IFE cannot be stable without them being fixed.

The module could also benefit from another round of usability review to help with priorities. I think there is a usability meeting or two each week that could be a good opportunity to get feedback on that:

There are two usability meetings every week! One at 7pm UTC on Tuesday while the other is at 7am UTC on Wednesday. Pick the best for your timezone. The meetings are at https://drupal.slack.com/archives/ux, get an invite at http://drupalslack.herokuapp.com/

(From @gaborhojtsy).

xjm’s picture

Project: Drupal core » Drupal core ideas
Version: 8.3.x-dev »
Component: inline_form_errors.module » Plan
Status: Active » Needs review

Also moving to the ideas queue for visibility.

dmsmidt’s picture

Issue summary: View changes
Wim Leers’s picture

Gábor Hojtsy’s picture

The usability review that @xjm called for in #62 happened 7 days after that comment. This issue was reviewed with @dmsmidt in the usability meeting on November 22nd with demos being provided of the bugs. Check out the recording in the first 31 minutes of https://www.youtube.com/watch?v=flpmTx0HPvE

The short summary is:

- the priorities identified in the issue summary look fine and their relative priorities make sense
- specifically for #2828092: Inline Form Errors not compatible with Quick Edit just not providing the inline form errors feature with inline editing sounded like a reasonable stop-gap (error displays with inline editing are already pretty broken and that is even a stable module)

I re-watched the recording and did not find any other high level feedback.

xjm’s picture

Thanks @Gábor Hojtsy.

I'm glad to see all the progress in the past couple months on the related issues. I hope to see more work in the next few weeks.

xjm’s picture

I guess the one thing I would add to #66 is that, from the release management perspective, a couple of the normal issues in the summary related to accessibility might be considered part of the accessibility gate, and therefore must-have. Some of the other normal issues might also be "Must" or "Should" have for the module to be stable since they are related to the usability gate, but I do agree with the current blockers listed being the top priorities.

dmsmidt’s picture

@xjm, could you more clearly define "some issues" so that we can keep our focus? Thanks in advance.

xjm’s picture

Yeah, thanks @dmsmidt. The three issues currently listed as blockers are definitely still the top priority, and what I really think we need to get done within the next few weeks. After that:

If we can't get some serious progress on those things, we'll need to hide the module from new installations and move development of the module back into contrib until the core blockers are fixed and the issues within the module are polished up as well. We do still want this feature in core if we can stabilize it, because we've definitely heard feedback on how it can improve Drupal's accessibility.

Edit: I put my scale for the accessibility example backwards. Fixed now.

dmsmidt’s picture

@xjm, thank you for the elaboration. Mostly I agree. Some feedback:

xjm’s picture

Status: Needs review » Needs work
Issue tags: +Needs issue summary update

Thanks @dmsmidt. Those points make sense to me.

Let's update the summary based on #70 and #71.

xjm’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs issue summary update

Actually I just did that. :)

xjm’s picture

Issue summary: View changes

Additionally, I think this module will need a full accessibility review once more of the issues are fixed. I added that to the summary as well, but clarified that it does not have the February 1 deadline.

xjm’s picture

Issue summary: View changes
xjm’s picture

Issue summary: View changes
xjm’s picture

Issue summary: View changes

Tried to clarify deadlines for specific issues as best I understand them currently, so that contributors can see which are more time-critical based on the release cycle.

The issues with a listed deadline of February 1 are issues to fix in core for IFE, rather than patches to IFE. If they are completed after February 1, they can be committed to 8.4.x instead of 8.3.x. Furthermore, if they are committed to 8.4.x before February 15, core committers may still consider backporting them to 8.3.x given their impact on this module. However, it will depend on the final patches, so that's why I've stated the deadline as February 1.

xjm’s picture

Issue summary: View changes

Hmm, #2509268: Inline errors repeated on child elements in module uninstall form appears to also be changing stable core rather than the IFE module. So, I've moved that one up to the Feb. 1 "should have" as well.

xjm’s picture

Issue summary: View changes
xjm’s picture

Oh, for more information on #77 and deadlines in general, see this recent policy clarification on core alphas and betas and the updated release cycle dates. (Just in case anyone is confused about where these deadlines are coming from.)

Sorry for so many separate little comments but hopefully this helps contributors.

hass’s picture

Have you fixed the issues that it was no longer possible for other modules to grab the form errors via drupal_get_messages() or a service? I need this errors in Google Analytics or we have a feature regression.

dmsmidt’s picture

@xjm thanks for the clarification.
@hass, this is unrelated to IFE. Form errors are stored in $form_state, keyed by the imploded#parents array. Just call $form_state->getErrors() to get your form errors.

hass’s picture

That is one of the design issues of this module. I need the messages in hook_page_attachments(array &$page). If this is not implemented this module needs work and is not ready for prime time.

xjm’s picture

@hass, can you file a separate child issue for #81 / #83? We can add it to the normal followup list for the issue if subsystem maintainers agree with the change. It's not a Feb. 1 deadline issue though.

dmsmidt’s picture

@hass, @xjm, yes a separate issue would help. Thus off-topic: as a workaround you could use a hook_form_alter(), get the errors, store them for the request and then expose them how you want in hook_page_attachments(). Using drupal_get_messages() may be a bit messy anyhow, since you would then need to set the messages again for the user to see them, right? Also, using that function you lack any context, you just get a bunch of errors. Lets discuss this further in a child issue.

dmsmidt’s picture

Issue summary: View changes

Marked some more issues that are problematic even without IFE.
Also note that we made progress with the must and should haves.

dmsmidt’s picture

@xjm, during the last usability meeting (17-01-2017) we agreed that #2687249: Improve inline form errors for file upload fields and therefore also #77245: Provide a common API for displaying JavaScript messages are no bugs. There is room for improvement but it's a nice-to-have. Our view changed because we noted that by fixing #2482783: File upload errors not set or shown correctly everything already works good enough!
I posted screenshots in the first and last mentioned issue. Do you agree, and if so, could you change the priorities accordingly?

TR’s picture

Regarding #71 which says "#2560467: Inline Errors not shown for container elements, currently nowhere in core we set errors on container elements a.f.a.i.k., so maybe just keep the priority at 'normal'."

This is not true. If you will read my comment from 2 years ago in the original issue (https://www.drupal.org/node/2509268#comment-10048238) you will see that back then I identified 13 places in core where we did this - repeating that examination today I still find 12 places (2 of which are in tests). Specifically, in these cases core calls setErrorByName($name, $message) with $name = '', expecting that the error is set on the page (container) rather than on a specific form element. This is a long standing pattern that has been copied throughout contrib. There are perhaps other instances in core where $name is explicitly set to a container element, but it will require more than a brief scan to determine this. A brief scan for setErrorByName() calls has some calls which sure look like they might be setting an error on a container, but I didn't investigate this further.

So I would argue that #2560467: Inline Errors not shown for container elements should go back in the "Should" category, as proposed by @xjm in #70, rather than downgrading it on the basis of the faulty conclusion from #71.

dmsmidt’s picture

@TR #88: thanks for your scrutiny.
I reread what you said earlier. As I understand it, the 13 places you found in core, are about setting errors on the complete form using an empty string as element name. This is indeed undocumented behavior, but no problem for IFE. There is a separate issue for that already: #2780209: Core relies on undocumented feature of FormStateInterface::setErrorByName(). I think the term 'container' is confusing. That issue is only about container elements ['#type' => 'container'] and not about the page container. I'll do a count of how many times this possible happens in core, and leave it in the related issue and update this comment accordingly.

Update:
As promised I did some research, I didn't find any usage of setting errors on elements with type 'container'. See:
#2560467-49: Inline Errors not shown for container elements

dmsmidt’s picture

Issue summary: View changes

Great, several issues are now RTBC due to the sprint weekend.
As per #87, demoted some issues.

dmsmidt’s picture

Actually great news, all 1 Feb deadline issues are fixed or RTBC.

SKAUGHT’s picture

@dmsmidt.
cheers good sir.

dmsmidt’s picture

Issue summary: View changes
xjm’s picture

#2509268: Inline errors repeated on child elements in module uninstall form and #2754977: Enhance formErrorHandler to include children errors on RenderElements are in! Great work on these issues during the Global Sprint Weekend.

#2346773: Details form element should open when there are errors on child elements is now unblocked and needs review, as does #2482783: File upload errors not set or shown correctly. Looks like we are a good track to stabilize the module and keep it in core.

As a reminder, #2346773: Details form element should open when there are errors on child elements and #2482783: File upload errors not set or shown correctly are now targeted for 8.4.x since we are preparing for the alpha release. If they are committed soon, we'll probably backport them to 8.3.x as well. (See the alpha allowed changes policyfor background information.) Then #2828092: Inline Form Errors not compatible with Quick Edit needs to be fixed or at least hotfixed within the module for it to be disabled with Quick Edit by March 1. So the priorities in order are:

  1. #2346773: Details form element should open when there are errors on child elements (committed before Feb. 13)
  2. #2482783: File upload errors not set or shown correctly (committed before Feb. 13)
  3. #2828092: Inline Form Errors not compatible with Quick Edit (before Mar. 1)

I agree with #87 (was on vacation before).

andrewmacpherson’s picture

I was reviewing the most recent work at #2346773: Details form element should open when there are errors on child elements and found some confusing comments (#18-32) about the related vertical tabs issue (#2531700: Fragment links to children elements in closed grouping elements don't work).

It seems #2531700: Fragment links to children elements in closed grouping elements don't work was marked as a duplicate of #2346773: Details form element should open when there are errors on child elements but there was no consensus on whether it was in scope there. The issue title and summary for #2346773: Details form element should open when there are errors on child elements don't include it, and it hasn't been mentioned for over a year.

Given that we've found a fix the details-open aspect described, it means we're on course to meet all of the "Feb 1st must-have" blockers here. (At least, as they are described in their issue summary.)

Shall we re-open #2346773: Details form element should open when there are errors on child elements as a follow-up? It hasn't been triaged as a must-have or should-have, but in any case we still have a March 1st must-have deadline for #2828092: Inline Form Errors not compatible with Quick Edit .

(I said the same thing over in comment #104 of #2346773: Details form element should open when there are errors on child elements.)

edit: corrected an issue number

dmsmidt’s picture

Issue summary: View changes

@andrew, good catch.
That issue is wrongly marked as a duplicate, the source of that problem is totally different than what is handled in #2346773: Details form element should open when there are errors on child elements.
I just manually tested it for closed details and the quick links don't work for closed grouping elements in general.
In any case we need to reopen that issue, and the product manger needs to indicate the priority. Let's discuss in the reopened issue further.
For now I added it as a should have.

xjm’s picture

Issue summary: View changes

Moved #2531700: Fragment links to children elements in closed grouping elements don't work to "Must" and #2848507: Indicate that grouping elements have child element errors for ux and a11y to "Should". Thanks @andrewmacpherson for testing and finding that this was still an issue!

xjm’s picture

There's understandably a lot of stress around this issue as we approach the beta deadline, so I'm going to post a brief update. The second-to-last core blocker for this has been committed, and the last one is under committer review. Many contributors have really pulled together over the past month to make that possible!

Contributors who are eager to help with this module can work on the issues listed as "By March 1" in the summary.

dmsmidt’s picture

@xjm thank you for the update!
@all: any help is greatly appreciated.

I will be at the DevDays in Sevilla (sadly it is later in March) polishing IFE, if we can keep this in.

kattekrab’s picture

@dmsmidt is there a way we can break up the remaining work to do into discrete tasks that more people can tackle?
Or would that just make things harder at this stage?

dmsmidt’s picture

Per @kattekrabs request.

Some smaller descrete tasks for the issues that have a March 1 deadline.

#2531700: Fragment links to children elements in closed grouping elements don't work todo's:
- Screenshots (but better gifs) that show the problem for all affected grouping elements (like fieldsets/details).
- Issue summary update based on latest comments
- JavaScript patch (I left a possible approach in the comments)

#2848507: Indicate that grouping elements have child element errors for ux and a11y todo's:
- Screenshots of the problem (for all grouping elements)
- Issue summary update, that clarifies the problem and the proposed solution (see comments)
- PHP part of patch: add an error class to grouping element
- CSS part of patch: style grouping elements that have an error

#2828092: Inline Form Errors not compatible with Quick Edit
We need some input about best way to solve this. There is a patch (quickfix), but we may need a data model change (Addition of render element property) to make it possible for contrib to disable IFE.
At least we need a functional test, that can already be written, even without the discussion being finished.

dmsmidt’s picture

Issue summary: View changes

As per #2482783-53: File upload errors not set or shown correctly removing from the deadline. Targeted for 8.4.x.

xjm’s picture

Issue summary: View changes

#2482783: File upload errors not set or shown correctly is still outstanding, but we are working on a plan there to come up with an internal hotfix that may be beta- or patch-safe, and then have an 8.4.x followup to use an improved API for file services. Updating the summary accordingly.

#2828092: Inline Form Errors not compatible with Quick Edit is the biggest concern right now for IFE.

Edit: Fixed link.

xjm’s picture

Issue summary: View changes

After reviewing what remains for this release, based on our policy, I think we can safely commit something to address the next issues for RC2 by March 13, so I am updating that in the IS. Those issues are still the top priority. Thank you so much to the contributors who are working tirelessly on them.

I also asked for some help from the product and usability team to get direction on the QuickEdit one and they will follow up there.

xjm’s picture

Issue summary: View changes

(Oops, fixing to be the commit freeze rather than the release date, March 13.)

dmsmidt’s picture

#2828092: Inline Form Errors not compatible with Quick Edit tests green! I left a comment about a follow up, @xjm what do you think, RTBC?

dmsmidt’s picture

And also #2531700: Fragment links to children elements in closed grouping elements don't work has functional JS test coverage and a fix, code reviews and manual tests would be great!

xjm’s picture

Issue summary: View changes

Thanks @dmsmidt. I added the followup #2856950: Add a possibility to disable inline form errors for a complete form to the summary.

xjm’s picture

Issue summary: View changes
Issue tags: +8.3.0 release notes

So the Quick Edit hotfix was committed and the only things outstanding for RC are #2531700: Fragment links to children elements in closed grouping elements don't work and #2848507: Indicate that grouping elements have child element errors for ux and a11y. Looking at the current patches on those two issues, it looks like they are also core bugs being exposed by IFE. I haven't reviewed closely enough to confirm this for sure, but it's likely, given how many other core bugs IFE surfaced in more severe ways that we fixed already. Based on that, I'm updating the summary to reflect that the RC2 deadline is not actually the best deadline for those.

Given that #2828092: Inline Form Errors not compatible with Quick Edit and all the other must-haves from the product team are in, I'm confident that Inline Form Errors will remain in core for 8.3.0! This was a very near thing and it's really inspiring how the community pulled together to fix the things that made this too risky to keep in core before. Thanks especially to @dmsmidt and the accessibility topic maintainers for their leadership on this issue, to the people who rallied sprinters around fixing the bugs, and to all the reviewers and patch contributors who got the prioritized issues done.

The next steps are:

  1. Complete the work needed to mark IFE as beta stability. Beta modules are considered API- and feature-complete, and provide BC and upgrade paths. I've identified one issue that I'd consider an API addition to complete before beta; I'll double-check the other issues later.
  2. Complete the work to mark IFE as a release candidate. This will mean all the critical (must-have) issues are solved, as well as most should-haves, and we think it's ready to be stable. I've indicated in the summary which I think those are (so far anyway).
  3. After final reviews and product, framework, and release manager signoff, make IFE a stable core module by moving it out of the experimental package in the 8.4.x prior to 8.4.0-alpha1.

Thank you everyone!!

I'm also tagging this for the 8.3.0 release notes (even though it's not complete) so that we can highlight the extensive improvements to IFE since 8.2.0 in the release notes and the CHANGELOG.

dmsmidt’s picture

Issue tags: +DevDaysSeville
dmsmidt’s picture

Component: Plan » Idea
Issue summary: View changes

Great to see that IFE is now alpha!
Although mentioned last in the 8.3.0 release notes' experimental modules list, it's a big win for ux and a11y!
Fixing IFE related issues still means fixing core issue that are present without IFE: double wins.
Let's get this stable now, it's very doable.

@xjm I would ping you, to have a say about #2482783: File upload errors not set or shown correctly after our discussion in Spain.

dmsmidt’s picture

Component: Idea » Approved Plan
SKAUGHT’s picture

cheers Drupal. alpha never sounded so nice.

Roya-Rateshtari’s picture

Hi
i was looking at https://www.drupal.org/core/experimental and I see the inline form errors deadline is 8.3.0-beta1. And I looked at https://www.drupal.org/core/release-cycle-overview and find out that the 8.3.0 was released on April 5, 2017. it seems the inline form errors has been passed-due. was the deadline extended? Which comments on this issue document the deadline extension? Does this issue summery date needs to be updated?

gnuget’s picture

It wasn't.

Accord with: https://www.drupal.org/core/experimental for Drupal 8.3.0-Beta 1 IFE was expected to have an alpha, and actually, they are on Alpha 2, so it is on time.

The table needs to be updated with what is expected from IFE for Drupal 8.4 though.

rootwork’s picture

@Roya-Rateshtari Dries mentioned this in the Driesnote -- because IFE got a substantial amount of work done, its status was continued into 8.4.x as an experimental module. So I think the deadline is probably 8.4.0's beta phase now, but someone with more authority should probably confirm that.

dmsmidt’s picture

Thanks for the interest. In #109 @xjm explains the status very well. IFE is alpha now and we need to be stable before Core reaches 8.4.0-alpha1 (13 weeks to go till the week of July 31, 2017). Any help is appreciated!