UI issue on nav-menus.php

[dilipbheda]

button border color issue, if users click on create menu button without enter the menu name.

STEP:
1) Go to Appearance -> Menus
2) Click on create menu button without enter the menu name.

I have attached an image for better understanding.

[dilipbheda]

Before apply patch

[dilipbheda]

After apply patch

[sabernhardt]

Custom link inputs with error border, before patch

[sabernhardt]

Custom link inputs with error border, after patch

[sabernhardt]

the OK buttons for post and comment dates correctly do not have the red border when the date/time is invalid (with or without the patch)

[sabernhardt]

@dilipbheda Thanks for the patch!

50686.patch​ successfully removes the extra red border from the "Create Menu" submit button when the menu name is empty. Likewise, that border is no longer on the "Add to Menu" button when a Custom Link's URL field is left empty. The date/time selection fields for posts and comments can also be inside the container with a form-invalid class, but the OK buttons are not within the container (so those do not need fixing).

However, the border is still on the Custom Link's "Link Text" input though the error only relates to the empty URL field. Technically, the selectors could have another :not() to make .form-invalid input:not(.button):not(#custom-menu-item-name), yet there probably is a better option.

I added the accessibility focus so the team could consider more substantial feedback for these errors, appearing in close proximity to the inputs. If that's outside the scope of this ticket, it could be considered on another.

I also noticed that the extra border occurred in a version earlier than trunk (5.4.2).

[afercia]

Related: [33109] / #32490 where a "\f534" dashicon was added to the styling of the invalid input fields. See for example the Add New User page. Seems that change missed various input fields in the admin that use the same styling. Also, in some pages the icon is completely off place, see for example the term.php page in the screenshot below.

The ​validateForm() function and similar implementations in core that set the form-invalid CSS class and only add a visual feedback are very old bits of code and show their age.

I'm not sure it is worth spending time to make validateForm() more accessible. A solid, comprehensive, user input validation API (see #47505) would require many more things than a border and some ARIA attributes. First of all, it would require a standardized ways to generate the input fields, see the ​Fields API project, which is in a dormant state. It should be used for the Settings API first and then made available to plugins and themes. These are big features that historically WordPress misses and would need some big effort. Hopefully they will get prioritized, at some point.

[afercia]

Add new user: invalid input fields styling

[afercia]

Edit terms: broken styling

[sabernhardt]

using more exclusive CSS selector .form-invalid input:not(.button):not(#custom-menu-item-name) and adding required attribute

[sabernhardt]

Thanks @afercia for looking into this. So it's probably not worth a big fix just for the menu pages.

Then I propose these smaller changes here:

1. editing the CSS selector to be stricter
2. adding the required attribute to the Menu name input (so at least the browser can give its usual notification)

[audrasjb]

Let's move it to milestone 5.6

[dschalk]

[I have tested this patch and can confirm it works as expected.
The problem where buttons incorrectly got the red form-invalid styling seems solved.

After patch:

I have performed a general smoke test to see if I could find any other forms with buttons that had issues but have not found any.

The styling issues mentioned by @afercia are not fixed by this patch, I would propose to create a new issue for these.

[audrasjb]

Indeed, let's commit the current patch and open a new one to handle styling issue.
I'll open a new ticket as soon as this one is fixed, and I'd like to milestone it for 5.6, so I'm also adding early keyword.

[helen]

I am famous for doing this but I am also very sorry in advance for it :) I think unfortunately in this case we do need to go at least a little bit wider for a fix that doesn't put us into a difficult place later. I don't think we need to go all the way into fixing validateForm from an accessibility point of view for the purposes of fixing this bug, but I think the correct approach here would not to be add a bunch of specific :not selectors to the CSS, but rather to add .form-required to the appropriate form inputs and use that existing naming.

That said, because it's existing naming and affects both CSS and JS, going that route will need a lot of testing. I think the initial approach for the menus screen would be to add .form-required to the specific inputs that should be highlighted (not on a parent element) and add .form-invalid .form-required { border } to forms.css. From there, you would need to test that validateForm doesn't interfere with menus because of the way it works, and that the new CSS doesn't adversely affect other elements that already use .form-required.

Does that make sense? I want to be clear that the patch isn't *bad*, it's just that I see this style of change as a bit of a red flag especially when it comes to trying to understand why it's this way in the future.

[sabernhardt]

After discussion during today's #accessibility bug scrub, we decided to wait on punting for another week, in case someone wants to try the JavaScript approach in time for 5.6.

[helen]

50686.diff​ is what I think a solution could be given what's currently in place. I tested in a few spots and didn't notice any new breakage. The basic idea here is that when .form-invalid and .form-required are on the same element, which is the generic case currently in places like Edit Category and Add New User, the border continues to be applied to any input and elements contained within that element. The new addition is nesting .form-required inside .form-invalid, which this patch adds to the nav menu screen, and then applying the border only to the .form-required element itself.

The issue on the Edit Category screen where the alert icon is out of place turns out to be due to the p.description underneath, and the fact that we are applying the icon as an ::after on the td instead of the input. I think that's probably due to not previously being styled as inside the input and that you could potentially have multiple inputs/selects inside the td but it might be more correct to attach that icon to the input/select at this point. That would be better addressed in a new ticket, however, since it's not actually a component of this specific issue.

Would like a little more testing for any forms with required elements that I may have missed before committing this.

[sabernhardt]

patch does not apply border to invalid publish time in Classic editor posts/pages

[sabernhardt]

@helen Thanks for trying the other approach.

With the patch, I noticed that these did not receive the border when one is invalid:

• post schedule (classic editor) - date/time fields
• page schedule (classic editor) - date/time fields

These fields do show the border when left empty:

• new user - both username and email address inputs
• new menu - name
• menu custom link - both URL and link text
• new category - name
• new tag - name
• edit category - name
• edit tag - name

I don't mind that the latest patch applies the red border around the custom link text input, though technically it is not required (that's why the previous patch had the second :not()). If a URL is submitted with empty custom text, the "Menu Item" fallback link text is not very helpful anyway.

Due to the time, this probably should be punted. However, if one :not() is acceptable, I could create a quick patch to add the required attribute to the first patch and let the custom link text receive the border when the URL is empty.

[helen]

@sabernhardt I didn't test the classic editor so the classes can be changed there as needed. As for the menu item name, I just had the patch match what was currently there but if it doesn't need to be highlighted because of the default name insertion it's a matter of not adding the .form-required class to it. Not sure if somebody else wants to pick up this patch, otherwise I'll come back to it later.

[sabernhardt]

removing "form-required" class from custom menu link text input but adding it to 5 date-time fields (for publishing posts/pages/comments)

[sabernhardt]

I checked the publish date fields with Comments for this patch, in addition to the fields I checked last time.

[sarahricker]

Request for more testing from @sabernhardt

[helen]

In 49283:

Administration: Better targeting for required form field highlighting.

Previously, any input or select inside of a .form-invalid wrapper would get the red border highlighting, including submit buttons which was not visually correct. This now only applies to form elements with a class of .form-required inside of the .form-invalid wrapper. It also continues to apply the border to elements with both classes (.form-invalid.form-required) as that is how some of the admin markup is structured.

Plugin authors may need to do the same sort of class application seen in this commit, i.e. add .form-required to certain form elements.

Props sabernhardt, dilipbheda, helen.
Fixes #50686.