Make WordPress Core

Opened 5 years ago

Last modified 5 weeks ago

#54171 assigned enhancement

Twenty Twelve: Replace frontend jQuery usage with Interactivity API

Reported by: flixos90 Owned by: flixos90
Priority: normal Milestone: Future Release
Component: Bundled Theme Version:
Severity: normal Keywords: has-patch
Cc: Focuses: javascript, performance

Description

Using jQuery for a few simple JS interactions in WordPress themes is a common performance problem, which is in most cases relatively straightforward to address: Vanilla JavaScript has for the better part of a decade included enough APIs that work across all popular browsers so that the remaining benefits of jQuery are around a few utility functions, that could be re-implemented in a few lines of code. Using jQuery for such simple use-cases results in around 80% more JS code being loaded in the frontend.

This ticket focuses on removing jQuery as a frontend dependency for Twenty Twelve - specifically the frontend, as for example in the Customizer jQuery is loaded anyway (and performance is a bit less of a concern there).

Change History (16)

This ticket was mentioned in PR #1683 on WordPress/wordpress-develop by adamsilverstein.


5 years ago
#1

  • Keywords has-patch added; needs-patch removed
  • Replace jQuery usage in Twenty Twelve frontend scripts with vanilla JS.
  • Don't require jQuery to be loaded in the frontend as a result of the above.

Trac ticket: https://core.trac.wordpress.org/ticket/54171

#2 @SergeyBiryukov
5 years ago

  • Summary Replace frontend jQuery usage with vanilla JS in Twenty TwelveTwenty Twelve: Replace frontend jQuery usage with vanilla JS

sgomes commented on PR #1683:


4 years ago
#3

Adding a note that after these changes, the JS will no longer work correctly in IE 11. I'm not sure whether this is a concern, given that on the one hand we've deprecated it, but on the other hand we may want to preserve this theme working correctly in older versions of WordPress and their supported browsers.

felixarntz commented on PR #1683:


4 years ago
#4

@sgomes Thanks for the review! I've updated the code accordingly.

#5 @flixos90
4 years ago

Note that the implementation for this depends on whether we will drop IE support in the theme or not. The discussion around this should be centralized in https://core.trac.wordpress.org/ticket/55126#comment:2, which does the same for Twenty Sixteen.

#6 @karmatosed
2 years ago

  • Keywords dev-feedback added

As this has been sat for a little while I wanted to check-in how people are feeling about progressing this to close or coming up with a different resolution.

#7 @adamsilverstein
2 years ago

  • Focuses performance added

#8 @karmatosed
2 years ago

@flixos90 just raising up if you would like to still work on this? If not happy to either:

  • Close this.
  • Look to get steps on who can take this on.

#9 @flixos90
2 years ago

@karmatosed This, like the other similar tickets, is still something I would like to pursue. This has gotten a bit stale, since at this point it would probably make more sense to replace jQuery with using the Interactivity API, rather than vanilla JS. But this ticket makes sense to still be used for that.

I saw the similar Twenty Sixteen ticket was closed recently, I'd like to eventually reopen it for the same reason.

cc @adamsilverstein

#10 @karmatosed
2 years ago

@flixos90 I am more than happy to reopen the Twenty Sixteen one also if you want to have ownership of it and move it on. Would that be something you want to facilitate?

#11 @flixos90
2 years ago

@karmatosed Thank you, yes that would be great. Please feel free to assign me as owner for now.

#12 @karmatosed
2 years ago

Amazing, thank you @flixos90 and please let me know if there are any other tickets like that I can re-open or assign to you. Whilst we are absolutely trying to focus down things in default themes, we shouldn't miss the opportunity to improve performance when you are offering to take it on.

#13 @flixos90
22 months ago

  • Keywords needs-refresh added; dev-feedback removed
  • Milestone Awaiting ReviewFuture Release
  • Summary Twenty Twelve: Replace frontend jQuery usage with vanilla JSTwenty Twelve: Replace frontend jQuery usage with Interactivity API

Per https://core.trac.wordpress.org/ticket/61027#comment:10, I'm going to update the purpose of this ticket slightly: With the Interactivity API available, we should focus these efforts on using the Interactivity API instead of vanilla JS as a replacement for jQuery.

As such, this will need a refresh. Please read the linked comment on the overarching ticket for additional context.

This ticket was mentioned in PR #12411 on WordPress/wordpress-develop by @adamsilverstein.


5 weeks ago
#14

  • Keywords needs-refresh removed
  • Replace jQuery usage in Twenty Twelve frontend scripts with vanilla JS.
  • Don't require jQuery to be loaded in the frontend as a result of the above.

Refreshed version of #1683 (the original branch lives on a fork that can no longer be updated), merged with current trunk. Trunk changes since the original PR - the deferred loading strategy for twentytwelve-navigation and the aria-expanded toggling on the menu button - are preserved in the vanilla JS implementation.

@adamsilverstein commented on PR #12411:


5 weeks ago
#15

Merged the latest trunk (clean, no conflicts) and fixed several runtime bugs found while re-reviewing the vanilla JS against the original review feedback on #1683:

  • getSiblingElements() iterated parentElement.children (an HTMLCollection, which has no forEach) and the focus/blur listeners were attached via getElementsByTagName( 'a' ).forEach() - both threw a TypeError at load, breaking the whole script.
  • The touchstart handler had lost its negation: it only acted when the item *already* had the focus class, so the first tap on a parent item navigated immediately instead of opening the submenu. Restored the original behavior (first tap opens, second tap navigates).
  • Removed the duplicate per-link touchstart listeners in favor of the single delegated handler on body, which also covers dynamically added menu items (as discussed in the original review).
  • Registered the delegated listener with { passive: false }, since browsers default to passive touch listeners on body and preventDefault() would otherwise be ignored.

Verified in a browser harness: menu toggle updates aria-expanded in both directions, focus/blur toggle the focus class on ancestor items, and the first tap on a parent item is prevented and opens the submenu while the second tap is allowed through. jshint passes.

@adamsilverstein commented on PR #1683:


5 weeks ago
#16

This branch lives on a fork that can no longer be updated, so the trunk conflicts here can't be resolved in place. A refreshed version of this PR, merged with current trunk and with the review feedback above incorporated, is now up at #12411.

Note: See TracTickets for help on using tickets.