Delay multiline calculations until changes have been applied to the DOM

[finkef]

Fixes #2041, #2049.

This PR fixes useMultiline not returning the correct value on initial render or paste. The problem lies in the fact that the DOM measurements are executed right after the store changes, but before the changes have been flushed to the DOM. We circumvent this by delaying the measurements until after the layout has been applied in the next frame.

[raineorshine]

Looks good!

I'm noticing a brief flash where the thought is rendered with the incorrect line-height, which is less than ideal, but to be expected given the architecture. Any ideas about how to mitigate this? Maybe render it invisible, take the measurement, and then re-render and make it visible? I understand if that is out of the scope of the original issue.

[finkef]

Just to confirm, you mean the flash when pasting, correct? Upon reloading I can't see any flash, the fade in seems to hide it nicely.

https://github.com/cybersemics/em/assets/6552366/bdd3b1b8-a7b3-446c-879c-0dc450869e67

---

I've been going back and forth about how to hide this properly. The tricky part is that no matter how we render it invisibly first (immediately in the editable, outside of the tree, etc.) and take the measurement, we need to maintain state about the measurement and somehow make it available within useMultiline.

Not the most idiomatic, but maybe the most practical idea:

• onPaste we synchronously and invisibly render the pasted value as a thought and measure the dimensions
• We store the dimensions in a vanilla JS map with value as key: { "https://github.com/...": { height: 123 } }
• We let the onPaste event bubble up and store the value in the editingValueStore
• Within useMultiline we use the pre-measured dimensions if available as basis for the calculations

@raineorshine If this sounds like a good way forwards to you, would be happy to give it a shot.

[raineorshine]

Just to confirm, you mean the flash when pasting, correct? Upon reloading I can't see any flash, the fade in seems to hide it nicely.

https://github.com/cybersemics/em/assets/6552366/bdd3b1b8-a7b3-446c-879c-0dc450869e67

In the video you can see it renders with the incorrect line-height the first time you paste, and then immediately it renders with the correct line-height.

On the second paste it doesn't occur.

I've been going back and forth about how to hide this properly. The tricky part is that no matter how we render it invisibly first (immediately in the editable, outside of the tree, etc.) and take the measurement, we need to maintain state about the measurement and somehow make it available within useMultiline.

Not the most idiomatic, but maybe the most practical idea:

• onPaste we synchronously and invisibly render the pasted value as a thought and measure the dimensions

• We store the dimensions in a vanilla JS map with value as key: { "https://github.com/...": { height: 123 } }

• We let the onPaste event bubble up and store the value in the editingValueStore

• Within useMultiline we use the pre-measured dimensions if available as basis for the calculations

We can't tie it to paste, because the problem exists on any first render, for example when expanding a thought and rendering its children for the first time.

The solution would need to be endogenous to the component. It could render initially with visibility: hidden, measure, then re-render with a useEffect. Or we could attach a ref element and directly set visibility: visible after the measure to avoid the React render cycle completely.

I'm still weighing the potential performance/UX cost of this. Thoughts are already re-rendered after the initial measurement, so the performance is probably the same. There's just the slight delay that is being introduced that affects the UX. Whether it is worth it to add a delay to the first render of all thoughts just to avoid the line-height flash on some thoughts, I'm not convinced.

[finkef]

We can't tie it to paste, because the problem exists on any first render, for example when expanding a thought and rendering its children for the first time.

You're right. Totally missed those cases. When expanding a thought though, it seems that the flash is hidden by the fade-in.

The solution would need to be endogenous to the component. It could render initially with visibility: hidden, measure, then re-render with a useEffect. Or we could attach a ref element and directly set visibility: visible after the measure to avoid the React render cycle completely.

I'm not sure about setting visibility: hidden at first. This would mean that we're also hiding placeholder values. When pasting a URL, I believe this would end up flashing as well.

[raineorshine]

I've looked at this with fresh eyes, and I think we need to approach this differently. Forget what I said about visibility.

Notice how useMutiline uses useLayoutEffect instead of useEffect. This actually allows the DOM to be measured and re-rendered before the browser repaints.

Call useLayoutEffect to perform the layout measurements before the browser repaints the screen

https://react.dev/reference/react/useLayoutEffect#measuring-layout-before-the-browser-repaints-the-screen

If we use useLayoutEffect properly, we shouldn't see a screen flash. By adding rAF, we essentially guarantee a screen flash (even if it is mitigated by the fade-in in most cases).

I think the original issue only affects paste. I believe that paste triggers updateMultiline through the editingValueStore. If so, then it's possible that that is why updateMultiline is getting triggered too early on paste, since editingValueStore, and ministores in general, trigger outside the React component lifecycle. Try finding something in the Redux state that changes on paste (and only on paste) that you can subscribe to (e.g. state => getThoughtById(state, simplePath)?.value, though that also changes on edit). We should be able to subscribe to some state change without having to resort to timeouts.

[finkef]

I think the original issue only affects paste. I believe that paste triggers updateMultiline through the editingValueStore. If so, then it's possible that that is why updateMultiline is getting triggered too early on paste, since editingValueStore, and ministores in general, trigger outside the React component lifecycle. Try finding something in the Redux state that changes on paste (and only on paste) that you can subscribe to (e.g. state => getThoughtById(state, simplePath)?.value, though that also changes on edit). We should be able to subscribe to some state change without having to resort to timeouts.

For this to properly work, I believe we can't use any store subscription at all and need to pass in the actual value as a dependency to the useLayoutEffect. I'll give this a try and see if that would have any performance implications.

[finkef]

Closing in favor of #2081.