owl icon indicating copy to clipboard operation
owl copied to clipboard

perfectly synchronising actual DOM update of components

Open seb-odoo opened this issue 3 years ago • 8 comments

We currently have an issue where 2 child components are meant to be updated at the exact same time, but it is not possible.

The particular use case is the chatter top bar, where the "send message" button becomes background-colored differently depending on a state (opened or closed) and at the same time clicking on it opens/closes the composer below it.

What we want is the button to change color at the exact same time as the composer appearing. What happens is that the button color update always comes after the composer (dis)appearing.

My investigation led me to remove all possible async point in the chain, to a point where both were updated in the same animation frame, in a what looks like to be a perfectly sync way. However it still wasn't enough, because apparently the vdom patching is slow enough (and doing separate DOM operations for each element) that it is still possible to notice the timing difference.

Odoo task for reference: https://www.odoo.com/web#id=2243468&action=333&active_id=1519&model=project.task&view_type=form&cids=1&menu_id=4720

seb-odoo avatar Mar 26 '21 11:03 seb-odoo

hmm, first of all, this issue is impossible. Owl is designed asynchronously, and you cannot force two child components to be updated at the same time. This is just not possible.

Also, and this is really weird to me, if there is no async work going on, they will however be updated in the same animation frame, and what you mentioned about vdom patching being slow and doing separate dom operations does not make sense to me. What do you mean?

ged-odoo avatar Mar 26 '21 12:03 ged-odoo

I know it's not possible in current state, but it is a real issue that we have and that we can't solve so I'm still opening an issue.

I'm not exactly sure why it happens the way it does, but I'm confident they are done in the same animation frame as I had logs everywhere (and there was just one request animation frame in the log), there's just a few ms difference between the 2 vdom patches (I guess CPU time). And that is enough for the browser (tested both chrome and firefox) to display one change noticeably before the other.

I still have my debug code on a branch, I can orient and further my research if there are specific points that you would like me to clarify.

seb-odoo avatar Mar 26 '21 14:03 seb-odoo

Here is an example of the current log after I click on the button, where it's very clear there is a single call to requestAnimationFrame.

You can refer to the following commit for where I put these logs: https://github.com/odoo-dev/odoo/commit/0e8f7ef5139210a28624cb50f9dbe2de996df199

image

seb-odoo avatar Mar 26 '21 14:03 seb-odoo

how do you check for the noticeable change? not with a debugger?

ged-odoo avatar Mar 26 '21 15:03 ged-odoo

Just with my eyes (debugger can even be completely closed to notice the issue), I'm pressing alt+m (which is the shortcut to click on that button when a chatter is open) and it's visible that they don't update at the same time.

Here's a video corresponding to what happens. (check the send message button and the composer area)

https://user-images.githubusercontent.com/38213504/112654614-dad6c180-8e4f-11eb-9c9b-cf799a0c0202.mp4

seb-odoo avatar Mar 26 '21 15:03 seb-odoo

@seb-odoo fun fact: there is a css transition rule on the button:

  transition: color 0.15s ease-in-out, background-color 0.15s ease-in-out, border-color 0.15s ease-in-out, box-shadow 0.15s ease-in-out;

in bootstrap.css... If you disable that, there is no issue as far as i can tell. Nothing that owl can do about this, the rendering is totally contained in a call stack (or several microtask ticks), but it is updated atomically on the screen.

ged-odoo avatar Mar 26 '21 15:03 ged-odoo

Oh so that might explain it. I double checked our CSS custom for animations but I forgot that we still use bootstrap for buttons. Sorry for the waste of time here (mostly mine actually), and thanks for looking into it!

I still need to figure out how in our code to handle the workarounds I had to do in my commit to actually ensure we get into a single animation frame for both, because it relies heavily on calling stuff at the right time, which is a bit too weak in my opinion.

So I would still consider it nice if we could have a clean way to express that both components should update together when they do. Currently I can work it through the parent if I remove the shouldUpdate, but it's just "luck" that the parent has to render as well in this case. If only the 2 children had to render I still would be blocked here.

seb-odoo avatar Mar 26 '21 15:03 seb-odoo

I actually think that the correct way is to simply render the parent. This will ensure that each children are patched onscreen at the exact same time. Finding a way to link components together seems complicated. But why not. I just don't see now how to express it properly, and how it could be implemented

ged-odoo avatar Mar 26 '21 15:03 ged-odoo