Menu title field does not clear invalid class

[ritukaushal5693]

When we create the new menu and do not give any menu name and click on create a menu button than validation is shown in Menu name option. When we Fill the name of the menu validation color not changed. It’s still showing in red color. And Validation link is not showing in second create menu button.

[ritukaushal5693]

good-first-bug

[welcher]

Thanks for reporting!

Based on how some other forms are handled in core ( ie. New User ) we should be clearing the red border onBlur which is not the case here.

[ashokrd2013]

@welcher

I have patch for this. Please check it.

[welcher]

@ashokrd2013 thanks for the patch!

Just a couple feedback items for you:

1. Please generate the patch from the root of the entire repo - the same level as the /src and /tests folders. This allows the patch be applied very easily and if there is a need for unit tests, they can be in the same diff.

2. Can we use _.debounce on the keyup event to reduce the number of times the callback gets run?

3. There are some coding standard issues in the patch. Have a read of ​https://make.wordpress.org/core/handbook/best-practices/coding-standards/javascript/#spacing .

4. It's a small thing but menuName = $( document.getElementById( 'menu-name' ) ) is more performant than menuName = $( '#menu-name' )- ​https://jsperf.com/wrap-an-element-or-html-collection-in-jquery

Does this patch also address the issue with the missing border on the Create Menu button as well? If not, no worries we can iterate on it.

Thanks for working on this!

[piyush9100]

I have changed in nav-menu.min.js to overcome this issues.

[harshbarach]

Hey @piyush9100 , I have checked your patch and it's working awesome. Greatly appreciated.

[ashokrd2013]

@welcher I have updated my patch as per your guideline.

[welcher]

@ashokrd2013 thanks for the update.

My only suggestion would be to make underscore a dependency when nav-menu is registered inscript-loader.php rather than calling wp_enqueue_script in nav-menus.php - ​https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/script-loader.php#L878

Beyond the change above, there are still some best practices issues in regards to whitepace for example !menuNameVal should be ! menuNameVal etc. Have a read here for more details - ​https://make.wordpress.org/core/handbook/best-practices/coding-standards/javascript/#spacing.

[welcher]

@piyush9100 thanks for the patch!

However, the .min files are created part of the build process. Can you move your changes to the /src file? Perhaps take a look at the patch that @ashokrd2013 has put together and see if you can add to it.

[ashokrd2013]

@welcher updated my patch

[welcher]

@ashokrd2013 I've added some change to your patch to ensure that we're defining the JS vars before using them and to add some more spacing for readability. Great work!

Originally, this ticket also called out only one button having the red outline after the submit. I don't think it should be there at all. It seems like awkward UX and looks horrible ( to my color blind eyes anyway ). As such, I think we should stop it from being added to the button at all.

This is not going to make it before 4.9 RC ( the deadline for bugs is today ) so let's try to get that added to this patch and we'll aim for the next release.

I'm going to assign this to @ashokrd2013 but if anyone wants to work on it ( looking at you @piyush9100 :) ) please feel free to!

[welcher]

Refreshed patch

[welcher]

@ashokrd2013 my last patch didn't have the latest code in it from master. I've refreshed it.

[ashokrd2013]

Removed red outline from button

[welcher]

Adding new css selector for invalid inputs

[welcher]

I've done a slight refactor on the approach here.

Now instead of adding the form-invalid class to the div that wraps the entire form, I've added a new input.invalid selector which is added/removed to the input directly. This should prevent any unintended side effects of changing CSS.

Also, I have added an inline comment in scripts-loader.php that references this ticket to explain why we now have underscore as a dependency for nav-menu.js as that is not consistent across scripts loaded in wp-admin.

I think this is ready to go for 4.9.1

[welcher]

Update patch to remove jshint error

[welcher]

Patch refresh post standards update

[desrosj]

@welcher Are you able to refresh 42321.6.patch​? It is no longer applying cleanly to trunk.

[welcher]

Refreshed patch

[welcher]

@desrosj patch refreshed.

[afercia]

The patch should use the input event and not keyup:

• keyup was generally used only to cover IE 8
• input accounts also when pasting with the mouse
• the menu name doesn't have a title attribute any longer so checking for menuName.attr('title') should be removed?
• misses some coding standards (mainly: spaces)

Also, not sure the comment in script-loader.php should be a docblock starting with /**.

[afercia]

42321.diff​ addresses the points above.
Also, I'd propose to keep using the form-invalid CSS class, as the new one on the single input doesn't account for the focus style: when invalid and focused, two box-shadows (red and blue) were applied to the field. We shouldn't change this part as it would require some unnecessary CSS refactoring. As far as I can tell, there are no concerns for unintended side effects as form-invalid is only applied to the menu name field parent.

[pento]

In 44680:

Menus: Remove error styling when the user addresses the error.

An error is triggered if a Menu Name isn't given when the user attempts to create a menu. When they go back and add a name, the error styling can be removed.

Props ashokrd2013, piyush9100, welcher, afercia.
Fixes #42321.