autoload-visible should not trigger injection for page elements above visible page top

[gyst]

Patterns/src/pat/inject/inject.js

Line 1004 in 64fdb9a

doTrigger = reltop <= $scrollable.innerHeight();

Scenario: I have a document with 100 previews, each of which has a trigger: autoload-visible. It also has a comment sentinel with a page fragment url that takes me directly to the bottom of the page. Jumping to the bottom of the page should only trigger the injection of the last page preview image; pages 2-99 do not need to be loaded since they are in "flyover country".

As a complication, when I then move away from this page via injection, this does not work, or rather it waits for a long time for all the 98 spurious preview injections to complete, before I can navigate away.

References:

• https://github.com/quaive/ploneintranet/issues/4512
• https://github.com/quaive/ploneintranet/pull/4528

[cornae]

I felt this one coming ;)

[cornae]

I see it's on @thet's name but I think it should be on my name.

[gyst]

I don't see how you can fix this in markup. IMO it needs a javascript change on the line I quoted.

[thet]

@cornae does this need a definition phase?

[cornae]

@gyst I can't currently fix this in markup. @thet Yes.

[thet]

@cornae I'd like to work on that this evening. Can you give this feature a thought this afternoon?
Although, currently I think this is just a "bugfix". If something isn't visible above the fold, it shouldn't be loaded, right?

[cornae]

Yes. I'll IM you.

[thet]

tnx

[thet]

I'm off this afternoon and will check my mails/messages later

[thet]

First attempt to fix this issue: #941
Second attempt to fix this issue: #942

The idea would be:

• Use an intersection observer to check if an element is in the viewport. Trigger the loading then.
• If an element is out of the viewport cancel a previous trigger.

The canceling would be done by aborting the jquery ajax call or by sending an abortsignal for fetch. for that to work we would need access to the ajax request object and that would need a refactoring of pat-inject use patternslib base class and proper scoping/encapsulation. pat ajax would return the ajax request object and pat inject could store it on the pattern instance. work has begun here: #943
During this effort we could switch from jQuery ajax to fetch and use async/await which would give us the possibility to await for an pat-inject call to be finished.
However, that needs a little more time.

Instead the PR #941 uses an intersection observer and a window timeout to schedule the triggering which can be cleared if the element is moved out of the viewport.
The delay is something we normally want to avoid, but it was already there with the original implementation, so this would be acceptable.

Status: the PR #941 needs more fine tuning and testing, currently it doesn't behave as it should.