Page MenuHomePhabricator

Dev: MobileFrontend should not use deprecated jquery.throttle-debounce inside ImageCarousel.js and ScrollEndEventEmitter.js
Closed, ResolvedPublic

Description

There is no util.throttle. This seems to block MobileFrontend removing this code or we will need to provide our own code. The solution for debounce is to use mw.util.debounce

$window
	.on( 'resize', apply2(
		util.debounce( 100, function () { eventBus.emit( 'resize' ); } ),
		$.throttle( 200, function () { eventBus.emit( 'resize:throttled' ); } )
	) )
	.on( 'scroll', apply2(
		util.debounce( 100, function () { eventBus.emit( 'scroll' ); } ),
		$.throttle( 200, function () { eventBus.emit( 'scroll:throttled' ); } )
	) );

Acceptance criteria

  • Replace $.debounce with mw.util.debounce
  • Replace $.throttle with a standalone function or a new mw.util function (if that will be added)

Event Timeline

Change 546350 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/extensions/MobileFrontend@master] Replace usage of deprecated jquery.throttle-debounce

https://gerrit.wikimedia.org/r/546350

Ammarpad triaged this task as Medium priority.
Jdlrobson changed the task status from Open to Stalled.Oct 28 2019, 6:20 PM
Jdlrobson removed Ammarpad as the assignee of this task.
Jdlrobson updated the task description. (Show Details)
Jdlrobson removed a project: Patch-For-Review.
Jdlrobson added a subscriber: Ammarpad.

Thank you @Ammarpad for taking care of the $.debounce issue. For $.throttle I'm keen to wait a little to see if a function gets added to mw.util before adding it to MobileFrontend.

Change 546350 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Replace usage of deprecated jquery.throttle-debounce

https://gerrit.wikimedia.org/r/546350

Jdlrobson renamed this task from Dev: MobileFrontend should not use deprecated jquery.throttle-debounce to Dev: MobileFrontend should not use deprecated jquery.throttle-debounce inside ImageCarousel.js and ScrollEndEventEmitter.js.Dec 9 2021, 5:45 PM
Jdlrobson changed the task status from Stalled to Open.

No util.throttle seems to be on the horizon so it makes sense to roll our own at this point.

No util.throttle seems to be on the horizon so it makes sense to roll our own at this point.

This was added two days ago in MW 1.38 in f4db95cea29143c5ca03dddb2ee493e7117c1406. Also mw.util.debounce now switches the parameters but the old signature remains compatible (25b8abcb7e51b1a573d85b2d07372a1d746a2658).

Thanks for the update @Seb35 ! That's great news.

Hopefully, I can get this work prioritized, but patches welcomed in the mean time from volunteers - I will review any patches within 24hrs Mon-Fri.

Change 771350 had a related patch set uploaded (by Sbisson; author: Sbisson):

[mediawiki/extensions/MobileFrontend@master] Replace deprecated $.throttle with mw.util.throttle

https://gerrit.wikimedia.org/r/771350

@Jdlrobson as you can see above I made a patch to address this. I was testing an extension on mobile and I got annoyed by the warning.

I had to bypass the pre-commit hook since I'm running node v16.10.0 and it's expecting v14.17.5 but npm test ran successfully. I ran npm run build and committed the changes in dist/ but I confess I don't know if that's how it's supposed to be done.

Change 771350 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Replace deprecated $.throttle with mw.util.throttle

https://gerrit.wikimedia.org/r/771350

I ran npm run build and committed the changes in dist/ but I confess I don't know if that's how it's supposed to be done.

We've got CI to check that you did it correctly so looks like this went fine.