The admin bar on the front end has reduced functionality and bugs when jQuery not being used

[afercia]

Twenty Nineteen [Edit: and also the next official theme Twenty Twenty] doesn't use jQuery on the front end. This is a welcomed effort to make Twenty Nineteen not depend on jQuery and use vanilla JavaScript instead.

Unfortunately, seems it hasn't been well considered that the WordPress admin bar has some functionalities that depend on jQuery.

The admin-bar.js file is split in two main parts: the original one uses vanilla JavaScript. The other part includes functionalities that were added over time and that depend on jQuery. A check for jQuery determines which part of the file is going to be used. As jQuery is undefined in the Twenty Nineteen front end, the old part of the file is used and many functionalities are missing.

Most importantly:

• keyboard interaction with the menu: pressing Enter on a menu top level item doesn't open the menu any longer and instead navigates to the link destination, making the menu unusable with keyboard (this is nothing new, see https://core.trac.wordpress.org/ticket/19088#comment:26)
• same happens on mobile: tapping a menu top level item triggers navigation instead of opening the menu, making the menu unusable
• hoverIntent on the menu top level items can't work without jQuery: it was added 8 years ago in [18488] / #18299 together with other improvements
• the search input field icon is missing

I can't think of good ways to fix this other than rewriting the whole admin-bar.js file in vanilla JavaScript. Worth noting a few functionalities are basically duplicated in the two parts. Other functionalities should be rewritten instead.

Considering that ​WordPress has officially ended support for old Internet Explorer versions since two years, this could be a good opportunity to make the admin bar use vanilla JavaScript.

Any thoughts and feedback very welcome.

[afercia]

Twenty Seventeen: pressing Enter opens the menu

[afercia]

Twenty Nineteen: pressing Enter doesn't open the menu and navigates to wp-admin/post-new.php

[afercia]

Mobile: tapping the menu triggers navigation instead of opening the menu dropdown

[afercia]

Missing icon within the search input

[afercia]

See also #34668, related to the admin bar menu when used with screen readers.

[afercia]

For some history on the reasons why the "click-to-open (Tab to select, Enter to open/close)" behavior was implemented on the admin bar menu, see #19088, specifically starting from ticket:19088#comment:34. Various opinions were then collected on a Make post, see ​https://make.wordpress.org/accessibility/2011/11/28/do-people-think-it-is-better-for-the/

[afercia]

One more thing to consider: the admin bar is rendered in the footer since (if I'm not wrong) its introduction in [15671]. Not sure why it's also moved with JS, see ticket:14772#comment:71 and see d.body.appendChild( aB ) in the admin-bar.js file.

Regardless, WordPress 5.2 is going to introduce the new wp_body_open hook. For the future, it would be worth considering to move the admin bar at the top of the markup, to make the visual order match the DOM order. See also #47053.

[afercia]

Discussed during today's accessibility bug scrub. Agree to move to the 5.3 milestone for research and exploration. Two possible options came out from the bug scrub:

• either enqueue jQuery in the front end when the admin bar is displayed
• or rewrite the whole file admin-bar.js in vanilla JavaScript (with no jQuery methods)

A first step would be: research if a non-jQuery equivalent of hoverintent.js exists.

[audrasjb]

First step: enqueue jQuery if the Admin Bar is displayed

[afercia]

A first step would be: research if a non-jQuery equivalent of hoverintent.js exists.

There's a plain JavaScript, MIT licensed, hoverintent see ​https://github.com/tristen/hoverintent

I think this would allow to refactor admin-bar.js and make it jQuery-free keeping the current functionalities. @azaozz do you think it is worth it? Twenty Nineteen (and I guess more themes in the future) don't use jQuery and it would be nice to respect their intent when logged-in users navigate the front end pages.

[westonruter]

Instead of using hoverintent to cause the dropdown to appear, a pure CSS alternative to this would be :focus-within: ​https://caniuse.com/#feat=css-focus-within

In the AMP plugin this is used to make the admin bar's submenus keyboard accessible: ​https://github.com/ampproject/amp-wp/blame/1.2.2/assets/css/src/admin-bar.css (instances of .hover were replaced with :focus-within)

The only apparent downside to this is the submenu appears immediately upon tabbing to the element, as opposed to hovering for a bit or hitting enter (which IMO is harder to discover/use?). But, it works when JS is turned off or the JS failed to load.

[afercia]

@westonruter yup, but the main purpose of hoverintent is to add that brief delay. See discussion on [18488] / #18299.

Also, focus-within has no support in Internet Explorer 11 which is still a browser that WordPress officially supports. It would require the ​PostCSS focus within and the ​focus-within polyfill to work, which doesn't seem ideal to me.

Still, there won't be any delay when moving the mouse over the toolbar: all submenus would appear immediately, which is a very annoying experience.

[afercia]

Worth noting also Twenty Twenty won't use jQuery. Twenty Twenty is in the works and there's an issue and a pending PR to move from jQuery to plain JavaScript. See:

​https://github.com/WordPress/twentytwenty/issues/12
​https://github.com/WordPress/twentytwenty/pull/101

It appears this ticket is even more relevant now. During last weekly accessibility meeting on Slack, @aristath and @dinhtungdu expressed interest in working on a patch. It would be great to have a solution in time for WordPress 5.3.

During the meeting it was also noted that in the responsive view the admin bar menu dropdowns are broken because the touchevents necessary to make them work are based on jQuery. Tapping a top level item in the admin bar menu triggers navigation instead of opening the dropdown.

[afercia]

Sorry I don't have bandwidth to work on this ticket. I shared a couple test patches I was working on a few months ago. They're just proof of concepts I was playing with, not ready for anything else other than testing. Maybe they can help to get started.

admin-bar-hoverintent.diff​ uses the no-jQuery hoverintent from ​https://github.com/tristen/hoverintent. Seems to me it works fine.

admin-bar-no-jquery-wip.diff​ is a work in progress I was testing to progressively remove all the jQuery bits and re-implement them in plain JavaScript.

[dinhtungdu]

47069.2.diff​ is my attempt to rewrite admin-bar.js using vanilla JS only without any external dependency. All features in the jQuery version have been ported.
APIs used in the file are checked with caniuse.com.

[audrasjb]

Great job @dinhtungdu !
Everything is working fine on my side 👍

[audrasjb]

Testing patch and everything works as expected

[audrasjb]

Looks like there is some coding standards errors though (I've got whitespaces warnings on my side).

[dinhtungdu]

47069.5.diff​ fixed some logic errors showed by jshint.

This patch needs cross-browser and cross-platform testing. Things to test:

Desktop:

• nojs class is removed after the DOM is loaded.
• Scroll page to top when clicking on the admin bar.
• Focusing on #adminbar-search adds .adminbar-focused to #adminbarsearch. Unfocusing removes it.
• HoverIntent: the hover event doesn't fire instantly but delayed for 180ms.
• When a hash is present in the URL, the admin bar is hidden.

Keyboard navigation:

• Pressing Enter on the Skip to toolbar button focuses the admin bar.
• Pressing Enter on the menu opens the dropdown.
• Pressing Escape on the menu and sub-menu closes the dropdown.

Touch devices:

• hover class is removed when the user touches outside the menu items.
• Tapping on the menu opens the dropdown menu. Tapping again close the dropdown.

[dinhtungdu]

47069.6.diff​ addresses WordPress JS coding standard and adds some minor tweaks.

[audrasjb]

As discussed during Accessibility bug scrub, let's commit the jQuery option for the moment and reopen it for a best solution in 5.4.

[afercia]

47069.7.diff​ takes the minimal approach and just enqueues jQuery as dependency of admin-bar.

For the future WP 5.4, the accessibility team strongly recommends refactoring admin-bar in plain JavaScript and make this ticket a task. For now, at this point of the 5.3 release, the minimal approach seems the only wise option.

This adds ~105 KB (minified but not gzipped) to the front end. Of course only when logged in and the user preference "Show Toolbar when viewing site" is on. Of course, theme that already use jQuery won't see any difference.

Worth reminding that for themes that don't use jQuery some of the functionalities of the admin bar aren't working. For example: hoverintent and, most notably, the admin bar menu on mobile is broken. It always worked this way, maybe because a few years ago it was pretty common for themes to always use jQuery.

Today, more and more themes don't use jQuery.

Twenty Nineteen and also the new Twenty Twenty don't. Also other teams in the ecosystem are more and more "jQuery free": I tested a few themes from 2016 and since they don't use jQuery, the admin bar is not fully functional.

In light of the release of Twenty Twenty this issue seems pretty urgent. I'd propose to bring this ticket to the attention of a larger audience during ​today's dev chat on Slack.

pinging @francina as 5.3 Release Coordinator
pinging @davidakennedy and @laurelfulford as maintainers of the Bundled Theme component
pinging @azaozz as 5.3 Core Tech focus lead
pinging @ianbelanger as 5.3 Default Theme Wrangler
pinging @anlino as 5.3 Default Theme Designer

[audrasjb]

Thanks @afercia, 47069.7.diff works fine.
Totally agree with everything shared above. We should work on this early during 5.4 new cycle. Thanks @dinhtungdu for working so much on this ticket 👍

By the way, @afercia’s current patch is ready to go. Re-adding commit keyword.

[afercia]

In 46440:

Accessibility: Script Loader: Add jQuery as dependency of admin-bar.

On the front end, themes that don't use jQuery make the admin bar fallback to the non-jQuery implementation. Some important features miss from the non-jQuery admin-bar.js part, for example hoverintent and, most importantly, the touch events for the mobile menu don't work at all.

Enqueueing jQuery is the simplest option for now. For the future, a complete rewriting of admin-bar.js in plain JavaScript is highly recommended.

See #47069.

[azaozz]

Sorry I couldn't review this earlier (been sick for a while). The reason the admin-bar doesn't have jQuery as dependency is that (at the time) some themes were/are adding their own versions of jQuery on the front-end, and it becomes "big mess" when we also add the version from core.

The "mess" mostly relates to jQuery plugins that may pick and use the wrong jQuery version, and fail.

That's why the admin bar js has a fall-back mode to work without jQuery. Ideally this mode should be fixed and not jQuery added in all cases.

Alternatively this can be fixed in the affected themes.

[afercia]

That's why the admin bar js has a fall-back mode to work without jQuery. Ideally this mode should be fixed

I think we all agree on this point. That's the reason why all the prevalent opinion on this ticket is that the best option is to refactor admin-bar.js in plain JavaScript and adding the "enhancements" that currently work only in the jQuery part. Most notably: hoverintent and the admin bar menu touch events on mobile. There is an initial patch from @dinhtungdu that hasn't been reviewed yet. I also have to note the complete lack of feedback from the toolbar component maintainers for the entire life of this ticket. This isn't greatly helpful.

Regardless, worth noting again that with Twenty Twenty the admin bar on the front end will be broken, much like it has been broken with twenty Nineteen for a year or so and with all the themes that don't use jQuery since forever. Honestly, this doesn't seem the best service WordPress can offer to users.

[dinhtungdu]

@afercia I didn't notice that we have a toolbar component. Can I promote myself to become a maintainer?

[afercia]

@dinhtungdu sure you can! Best place to ask is next weekly dev chat on Slack on Wednesday at 20:00 UTC :)

[audrasjb]

RC1 is approaching. Closing this ticket as fixed for milestone 5.3.

[azaozz]

Looking at the possible problems with always enqueueing the core version of jQuery in all themes, this should be reverted for now. If it is needed for a particular theme, lets add it in the theme.

[afercia]

Agreed to revert this change for now, as there are valid concerns for potential jQuery versions conflicts.

Not sure this should be a themes responsibility though. Let's revisit a best approach in the next release cycle, preferably refactoring admin-bar.js in plain JavaScript so that all the functionalities are available regardless of jQuery.

[afercia]

In 46550:

Accessibility: Script Loader: Remove jQuery as dependency of admin-bar after [46440].

A better approach needs to be explored, as there are valid concerns for potential conflicts between different jQuery versions added by themes or plugins.

See #47069.

[azaozz]

It would be great if work continues to make the admin bar work without using jQuery. 47069.6.diff looks good but seems to need a bit more work and more testing.

Setting this tentatively for 5.3.1 as it would be a nice bug fix for some themes.

[dinhtungdu]

@azaozz Yeah, 47069.6.diff​ definitely needs more testing. I'm more than welcome to receive feedback and provide fixes for it :)

[afercia]

[46550] brought in some unrelated, unintended CSS changes. They pertain to #47477 which is unrelated to this ticket.

47069.8.diff​ partially reverts [46550] removing all the CSS changes. Note: I'd still recommend to commit the CSS changes, see https://core.trac.wordpress.org/ticket/47477#comment:67 but they should be agreed on and anyway pertain to #47477 :)

Being in Release Candidate period, 47069.8.diff​ needs a sign-off from another committer.

[azaozz]

Replying to afercia:

47069.8.diff looks good, +1 to revert the accidental changes during RC.

[afercia]

In 46559:

Script Loader: Partially revert [46550] as it brought in unrelated CSS changes.

[46550] was meant to revert [46440] but it also merged some unrelated CSS changes.

See #47069.

[azaozz]

Replying to dinhtungdu:

Great! :)

Some initial thoughts:

• Perhaps it would be good to "extract" the native hoverIntent into a separate file that can replace the jQuery based version? It's not necessary to be an "exact copy" of how it works with jQuery, just needs to have the same or very similar functionality.

• I'm not sure about the need for scrollToTop(). Smooth scrolling is now native functionality in most browsers: ​https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoView. Not sure we need to add that with js.

[dinhtungdu]

Replying to azaozz:

• Replacing hoverIntent: I agree! Only 3 files are using hoverIntent including admin-bar.js. I will work on replacing it this week.
• scrollIntoView is an experimental API, and many browsers don't have support for smooth behavior including Safari and iOS Safari/Chrome. So we should use scrollToTop() for now.

[azaozz]

many browsers don't have support for smooth behavior including Safari and iOS...

Yeah, I know. Safari is the new IE6! :) Eventually it will get added there. Doing full-page animations in js is quite CPU-intensive and may have the opposite effect, get so ridiculously choppy.

Thinking it would be a better option to leave that for the native browser implementation, then when this is added to Safari, it will "just work" there too.

Only 3 files are using hoverIntent..

Yes, but one of them is common.js that gets loaded everywhere in wp-admin. So hoverIntent is available everywhere too.

[dinhtungdu]

Replace jQuery hoverIntent with vanilla JS hoverintent. Use scrollIntoView for smooth scrolling.

[azaozz]

Looking at 47069.9.diff, works pretty well in the admin bar.

We cannot remove the old jQuery hoverIntent.js. It is likely used in a lot of plugins and expected to always be there as it is loaded on all wp-admin screens. The options there are:

1. Add a jQuery wrapper for the new hoverintent.js (and load it with the old hoverintent script-handle so it stays compatible).
2. Keep the old hoverIntent.js and add the new, no-dependencies one.

Thinking that perhaps the second makes more sense as there will be no eventual compatibility issues from adding a wrapper.

[azaozz]

In 47069.10.diff: Virtually the same as 47069.9.diff except it doesn't remove the old jQuery based hoverIntent.js.

[dinhtungdu]

@azaozz I agree that we need to keep the jQuery hoverIntent, but I think we should use the vanilla JS version for menu and revision to prevent loading 2 versions of hoverIntent in the admin dashboard.

Update: 47069.11.diff​ use vanilla JS hoverintent for admin menu and revision.

[afercia]

Keep the old hoverIntent.js and add the new, no-dependencies one.

Seems the most reasonable approach to me. It would be nice there was a deprecation policy for this kind of things :)

[azaozz]

Replying to dinhtungdu:

@azaozz I agree that we need to keep the jQuery hoverIntent, but I think we should use the vanilla JS version for menu and revision to prevent loading 2 versions of hoverIntent in the admin dashboard.

Not sure that's "wise" :)

As I mentioned before:

Only 3 files are using hoverIntent..
Yes, but one of them is common.js that gets loaded everywhere in wp-admin. So hoverIntent is available everywhere too.

Currently the jQuery hoverIntent is loaded everywhere in wp-admin. Some plugins probably assume it is "always there". For best back-compat we cannot stop loading it...

Ideally we should make a jQuery "wrapper" for the new hoverintent.js (and load it with the old hoverintent script-handle so it stays compatible). This doesn't seem like a particularly complex task, such wrapper may already exist somewhere.

Given the short time left for the 5.3.1 milestone, thinking for now we should continue to load both hoverintent versions, and look at making a wrapper in 5.4.

@dinhtungdu could you test/confirm 47069.10.diff works as expected please :)

[dinhtungdu]

Replying to azaozz:

Currently the jQuery hoverIntent is loaded everywhere in wp-admin. Some plugins probably assume it is "always there". For best back-compat we cannot stop loading it...

I see :)

@dinhtungdu could you test/confirm 47069.10.diff works as expected please :)

I tested it and I can confirm that it works :)

[azaozz]

In 46872:

Fix the admin toolbar js when jQuery is not present and replace the jQuery based hoverIntent.js with a native implementation. Introduces the "hoverintent" (no dependencies) package.

Props dinhtungdu, audrasjb, azaozz.
Fixes #47069.

[azaozz]

Reopen for the 5.3 branch.

[azaozz]

In 46873:

Fix the admin toolbar js when jQuery is not present and replace the jQuery based hoverIntent.js with a native implementation. Introduces the "hoverintent" (no dependencies) package.

Props dinhtungdu, audrasjb, azaozz.
Merges [46872] to the 5.3 branch.
Fixes #47069.

[SergeyBiryukov]

In 46874:

Coding Standards: Fix WPCS and JSHint issues in [46872].

See #47069.

[SergeyBiryukov]

In 46875:

Coding Standards: Fix WPCS and JSHint issues in [46872].

Merges [46874] to the 5.3 branch.
See #47069.

[azaozz]

There seem to be some errors in the "jQuery free" admin-bar.js. Caused by the way jQuery silences errors when a particular node doesn't exist. For example: $( '#some-element' ).on( 'click', ... ) will not throw error when a node with that ID doesn't exist but the equivalent document.getElementById( 'some-element' ).addEventListener() will.

Patch coming up.

[dinhtungdu]

@azaozz I see. I didn't test cases when users filter the content of the admin-bar 😬.

I'm thinking about checking variables before attaching events to them but it doesn't seem to be the best solution.

Update: the logout menu is not checked before using.

[azaozz]

Replying to dinhtungdu:

I'm thinking about checking variables before attaching events to them but it doesn't seem to be the best solution.

Yeah, it's either that or perhaps wrapping the whole thing in a try {} catch () {}. Checking node existence seems cleaner, perhaps? :)

Also not really sure about the Element.prototype.matches() polyfill: ​https://developer.mozilla.org/en-US/docs/Web/API/Element/matches. It's not a bad idea, but might bring some compatibility issues eventually?

This will run on the front-end for logged-in users. Generally the browsers they use should conform to the minimal browser support for wp-admin (IE11, newer Edge, Chromium and Firefox) but having some "error silencing" for old browsers (to emulate jQuery behaviour) may be a good idea.

For example looking at the browser compat table at the bottom of ​https://developer.mozilla.org/en-US/docs/Web/API/Element, there's no window.Element for old Edge and Chrome for Android.

[azaozz]

In 47069.12.diff:

• Silence errors (similarly to jQuery) when adding event listeners.
• Add "fallbacks" in functions that use element.classList.
• Some coding standards and readability fixes.

@dinhtungdu sorry for the late patch but could you please have a look at this asap so it can be added tomorrow :)

[dinhtungdu]

Replying to azaozz:

@dinhtungdu sorry for the late patch but could you please have a look at this asap so it can be added tomorrow :)

@azaozz It's 9 am here so it's no problem :D. The patch works great for me. I have some questions about the code:

• Why do we need to check for window.Element exist or not?
• The comment for hasClass function is not updated.
• The indentation of some functions is too deep for me, should we try to fix them with early return style? (I can patch). IMO, the small indentation level increases readability.

[dinhtungdu]

In 47069.13.diff​ :

• Change version in comment to 5.3.1.
• Update documentation for hasClass function.

[azaozz]

Replying to dinhtungdu:

@azaozz It's 9 am here so it's no problem :D. The patch works great for me. I have some questions about the code:

Ah, that's great! :)

Why do we need to check for window.Element exist or not?

Yeah, probably not needed especially after it tests for querySelectorAll at the top (added it mostly for completeness after looking at the (big) compatibility table at ​https://developer.mozilla.org/en-US/docs/Web/API/Element).

The indentation of some functions is too deep for me, should we try to fix them with early return style?

Sure, lets reduce it. Removing the window.Element check and returning early in few functions should reduce indentation :)

Patch coming up.

[azaozz]

In 47069.14.diff: remove check for window.Element and reduce indentation in few places by returning early.

This seems ready, @dinhtungdu could you have another look please and lets add that :)

[westi]

Replying to azaozz:

In 47069.14.diff: remove check for window.Element and reduce indentation in few places by returning early.

This seems ready, @dinhtungdu could you have another look please and lets add that :)

This works well on my test site with TwentySeventeen which was previously showing errors.

[dinhtungdu]

@azaozz Tested and everything works as expected.

@westi Do you remember what those errors are?

[westi]

Replying to dinhtungdu:

@azaozz Tested and everything works as expected.

@westi Do you remember what those errors are?

TypeError: adminBarLogout is nulladmin-bar.js:131:3
    <anonymous> .../wp-includes/js/admin-bar.js?ver=5.3.1-RC1-46877:131

Running the 5.3 Branch from svn with SCRIPT_DEBUG enabled

[azaozz]

@westi @dinhtungdu thanks for looking at such short notice! Lets add this :)

[azaozz]

In 46883:

Fixes in admin-bar.js:

• Silence errors when a node doesn't exist similarly to jQuery.
• Add "feature testing" and fallbacks for old browsers as this may run on the front-end.
• Improve inline docs.

Props dinhtungdu, azaozz.
Fixes #47069.

[dinhtungdu]

Replying to westi:

TypeError: adminBarLogout is nulladmin-bar.js:131:3
    <anonymous> .../wp-includes/js/admin-bar.js?ver=5.3.1-RC1-46877:131

Thanks! That's my mistake and azaozz fixed it.

[azaozz]

Reopen for merging to the 5.3 branch.

[azaozz]

In 46884:

Fixes in admin-bar.js:

• Silence errors when a node doesn't exist similarly to jQuery.
• Add "feature testing" and fallbacks for old browsers as this may run on the front-end.
• Improve inline docs.

Props dinhtungdu, azaozz.
Merges [46883] to the 5.3 branch.
Fixes #47069.

[aduth]

With the changes in r46872 (notably to the package-lock.json file), I see local changes on a fresh clone of wordpress-develop after running npm install. Unsure if this differs based on the version of npm used, but in my case I'm using the latest version (6.13.4).

At least on observation of the diff, it appears to be a simple shift via alphabetization.

diff --git a/package-lock.json b/package-lock.json
index bf9b85069a..89bf380ace 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -10731,6 +10731,11 @@
 			"integrity": "sha512-7T/BxH19zbcCTa8XkMlbK5lTo1WtgkFi3GvdWEyNuc4Vex7/9Dqbnpsf4JMydcfj9HCg4zUWFTL3Za6lapg5/w==",
 			"dev": true
 		},
+		"hoverintent": {
+			"version": "2.2.1",
+			"resolved": "https://registry.npmjs.org/hoverintent/-/hoverintent-2.2.1.tgz",
+			"integrity": "sha512-VyU54L1xW5rSqpsv/LJ6ecymGXsXXeGs9iVEKot4kKBCq5UodSAuy3DqX686LZxEpaMEfeCHPu4LndsMX5Q9eQ=="
+		},
 		"hpack.js": {
 			"version": "2.1.6",
 			"resolved": "https://registry.npmjs.org/hpack.js/-/hpack.js-2.1.6.tgz",
@@ -13217,11 +13222,6 @@
 				"jquery": ">=1.7"
 			}
 		},
-		"hoverintent": {
-			"version": "2.2.1",
-			"resolved": "https://registry.npmjs.org/hoverintent/-/hoverintent-2.2.1.tgz",
-			"integrity": "sha512-VyU54L1xW5rSqpsv/LJ6ecymGXsXXeGs9iVEKot4kKBCq5UodSAuy3DqX686LZxEpaMEfeCHPu4LndsMX5Q9eQ=="
-		},
 		"jquery-hoverintent": {
 			"version": "1.8.3",
 			"resolved": "https://registry.npmjs.org/jquery-hoverintent/-/jquery-hoverintent-1.8.3.tgz",