-
Notifications
You must be signed in to change notification settings - Fork 23
Add external hooks to now {visible,hidden} algorithms #47
Conversation
/cc @mounirlamouri |
Thanks @yoavweiss :) |
First.. THANKS to Yoav for writing this first draft. Now.. read the following to understand why I haven't yet made progress. I'm curious on the thinking around these and how we landed on this PR. This update seems to allow a synchronous hook. This does solve the request.. but.. I'm not certain it is the proper design on first glance without understanding the features we are adding it for. How are they implemented? Do we have a place in a spec intends to reference this hook to help us reason about whether this is a synchronous execution hook OR should be a hook to start our processing model OR should we triggered an enqueued task for other subscribers? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please reply to comments and tag me. :-)
@toddreifsteck - The main user for this change is the Screen Orientation change algorithm, which seems to be using the sync hook to register async actions. Maybe we can trigger the hook async to begin with? @mounirlamouri - thoughts? |
Running the hooks sync has the benefit of allowing sync hooks if needed and in practice avoid issues of re-entrency. |
Now that I've read the consumer specification, I think the Screen orientation spec could be improved by altering the animation frame tasks to be a step during render steps that checks visibility and executes at that point. As written, if a state change from visible to not visible occurs, that will cause 2 sets of steps to run which seems suboptimal. If re-written that way, I don't believe a new hook is needed in Page Visibility. @mounirlamouri What do you think? |
Said another way.. each animation step needs to check 2 things: 1. Did the orientation change from last time? If so, fire that subset of steps. 2. Did the tracked state of visibility change from false to true? If so, run the visibility transition from false to true set of steps. (If the visibility flipped from false to true to false within a frame before the animation steps, I don't think we want the animation steps to execute the transition.) Does what I'm saying make sense? |
I don't think we should optimise the number of steps. Hooking into the animation steps would have us run a check on every frame which I doubt any UA would implement. Unless I misunderstood? |
K, understood. I'm signing off and merging this. I feel it would be "cleaner" to add a call from Page Visibility to invoke Screen Orientation but I don't have a reason to block the "register/execute" spec design. :-) I've also added an issue to Screen Orientation to track what seems to be a processing model issue. w3c/screen-orientation#179 Thanks! |
Were these definitions intentionally not exported with |
The latest published version on TR has them as exported, but they have not been picked up yet by WebRef for some reason. I'll see if I can find out why. |
@rakuco, filed https://github.com/w3c/respec-web-services/issues/200 ... will try to fix it there. |
Fixes #40
Preview | Diff