Nominating for current sprint

mediawiki.util.js maybe?

Change 267058 had a related patch set uploaded (by Jhobs):
Upstream isElementInViewport from MobileFrontend

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

Have commented that I think this requires a little more thought and wider input. I suspect this will carry over to next sprint which is fine - upstreaming takes time.

I have also added my two $monetaryUnits.

Yeah, I saw, thanks for your comments. I'm going to let the discussion ride out a bit (chiming in myself as well of course) before uploading any new patch sets.

Incidentally, suspicion that this may happen is also why I haven't written the MF & Gather patches yet. ;)

I don't know why, but it posted my previous comment twice... hence the now-deleted one.

@jhobs any updates on this?

@jhobs what @bmansurov said on e-mail - you should fix it up and not wait for more reviews.

I think my phrasing in the email was poor. I was not suggesting halting development, I simply meant I hope more people look at it soon in addition and it gets +2'd quickly after all the minors are taken care of.

@jhobs, sorry for misunderstanding.

Change 269914 had a related patch set uploaded (by KartikMistry):
New upstream release

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

Change 269915 had a related patch set uploaded (by KartikMistry):
New upstream release

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

Change 269916 had a related patch set uploaded (by KartikMistry):
apertium-dan-nor: New upstream release

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

Is there any way to disconnect those 3 patches by KartikMistry from this task? They're not related at all (and appear to be duplicates).

To avoid confusion with the above unrelated patches the patch that needs review is https://gerrit.wikimedia.org/r/267058

Was some useful input from @Krinkle
Now waiting on a follow up from @jhobs

QUnit is complaining.

In T124317#2061414, @bmansurov wrote:

QUnit is complaining.

I've run the test suite locally several times and can't find any issues. It appears to me that Jenkins is failing.

FYI php notice in https://integration.wikimedia.org/ci/job/mediawiki-extensions-qunit/32297/consoleFull

(Always read the logs)
rebase or recheck should fix

In T124317#2063383, @Jdlrobson wrote:

FYI php notice in https://integration.wikimedia.org/ci/job/mediawiki-extensions-qunit/32297/consoleFull

(Always read the logs)
rebase or recheck should fix

I read the logs, but it was the same in PS 12 & 13 even though I rebased in 13, so I too thought maybe a recheck would work, to no avail. Just rebased again, hopefully it'll work this time.

The error that keeps being thrown is

PHP Notice: Cannot find site jenkins_u0_mw in sites table [Called from Wikibase\Client\WikibaseClient::newSiteGroup in /mnt/jenkins-workspace/workspace/mediawiki-extensions-qunit/src/extensions/Wikidata/extensions/Wikibase/client/includes/WikibaseClient.php at line 597] in /mnt/jenkins-workspace/workspace/mediawiki-extensions-qunit/src/includes/debug/MWDebug.php on line 300

which refers to a file that I did not touch in the patch. I have no idea how to proceed from here.

Change 267058 merged by jenkins-bot:
Upstream isElementInViewport from MobileFrontend

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

This cannot be considered done until the core function is used by MobileFrontend and friends and the code in those extensions is cleaned up.

Change 282194 had a related patch set uploaded (by Bmansurov):
Remove the isElementInViewport function

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

Change 282196 had a related patch set uploaded (by Bmansurov):
Remove the isElementInViewport function

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

QuickSurveys and RelatedArticles have been covered in the above two patches. MobileFrontend has already been taken care of.

Change 282194 merged by jenkins-bot:
Remove the isElementInViewport function

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

Per T130849, isElementInViewport is an anti-pattern, because it forces a layout. I'd really rather see it removed entirely rather than standardized.

In T124317#2196088, @ori wrote:

Per T130849, isElementInViewport is an anti-pattern, because it forces a layout. I'd really rather see it removed entirely rather than standardized.

It's worth noting that the article you point to recommends "All calls to get any calculated dimension from the DOM should be cached or avoided."
We could certainly do the former and I would argue we can do it far more easily with the mediawiki.viewport library.

That said, this is an essential component of the lazy loading images solution. I'm curious to whether you know of an alternative for achieving lazy loading images without being able to test whether an image has been scrolled into view.

In T124317#2196567, @Jdlrobson wrote:

In T124317#2196088, @ori wrote:

Per T130849, isElementInViewport is an anti-pattern, because it forces a layout. I'd really rather see it removed entirely rather than standardized.

It's worth noting that the article you point to recommends "All calls to get any calculated dimension from the DOM should be cached or avoided."
We could certainly do the former and I would argue we can do it far more easily with the mediawiki.viewport library.

That said, this is an essential component of the lazy loading images solution. I'm curious to whether you know of an alternative for achieving lazy loading images without being able to test whether an image has been scrolled into view.

Right -- that's a good point. The use-cases I was thinking of were feature usage analytics.

Change 282196 merged by jenkins-bot:
Remove the isElementInViewport function

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

Given the discussion on this ticket and T130849, the Intersection Observer spec is definitely worth tracking.

Change 269914 merged by Alexandros Kosiaris:
apertium-nob: New upstream release

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