Skip to content
This repository has been archived by the owner on May 26, 2022. It is now read-only.

New state: "hold" #25

Open
sgulseth opened this issue Sep 23, 2015 · 7 comments
Open

New state: "hold" #25

sgulseth opened this issue Sep 23, 2015 · 7 comments

Comments

@sgulseth
Copy link
Contributor

Hi,

we want to introduce a new state integer, "hold". This is meant to be used to halt/pause rendering of an container, for example to lazy load ads when the user scrolls.

If we set the frame state to '40' on queue host will think it is resolved. When we want to render it we can then set the state to '0' again. This solution does not require additional code, just a reserved state-integer which is higher than resolved.

@gregersrygg
Copy link
Member

What is the benefit of introducing "hold" instead of waiting to trigger render() until the banner should render?
We should also think of ways to solve this as a plugin. Not sure it's possible with todays architecture.

@sgulseth
Copy link
Contributor Author

We have written a lazyload plugin, https://gist.github.com/sgulseth/d94a7eb26ed40ba48d85.
With this approach the plugin is plug-and-play.

@gregersrygg
Copy link
Member

Nice :) So you don't need "hold" anymore?

It works fine to call window.gardr.render(item.name); from a plugin, but I think garðr should have a way plugins can call render on an item without accessing window.gardr.

This almost sounds like a different kind of plugins than those we've made so far, but it's hard to see a new pattern yet. The others listen for events and do actions based on those, but this one needs to send a command to host. Any ideas? Have you come across other plugins that need host or ext to do something?

@sgulseth
Copy link
Contributor Author

I don't "need" hold, but this plugin exploits the iframe isActive(https://github.com/gardr/host/blob/master/lib/state.js#L69-L71). The method checks for if state is an int larger than 10 then return true. and in the manager render-method(https://github.com/gardr/host/blob/master/lib/manager.js#L299-L305) it only render if isActive returns false.

So as it is today, it will work, but it would be great if we could reserve the "hold" state for further updates, so we don't introduce any bugs if you suddenly changes the behaviour. The lazyloading of ads will be one of the core functionality in the ad loading for both VG and E24.

@sveisvei
Copy link
Contributor

sveisvei commented Oct 9, 2015

@sgulseth What would this change do, add pause() and resume() methods?

@sgulseth
Copy link
Contributor Author

@sveisvei yes, add an state which pauses the rendering of the ad

@sveisvei
Copy link
Contributor

One issue we would have to solve if introducing pause and resume, is renderAll render-priority will need to account to pause.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants