Refactor the Tooltip to avoid full re-rendering.

[afercia]

Splitting this out from #43952, see there for a an actual use case.

Turns out that setting a Tooltip conditionally may trigger a full re-rendering of the button the tooltip is set to.

A full re-rendering is less than ideal for a couple reasons:

• a11y: it triggers a focus loss
• it may make e2e tests fail

There are legitimate cases where we would want to set a Tooltip conditionally, for example:

• A button has visible text.
• The button is associated with a keyboard shortcut and we want to visually show the shortcut within the tooltip.
• However, the button is initially disabled and gets enabled only after some action.
• Ideally, when the button is disabled, it should not show any tooltip because:
  • the button has visible text
  • the button doesn't do anything so showing a keyboard shortcut that doesn't do anything is pointless and misleading for users
• When the button gets enabled, at that point it should show a tooltip.

Right now, the only workaround is to force the tooltip to always be rendered by setting the showTooltip prop to true. However, it doesn't make much sense to show both visible text and a tooltip. For example, a button with text 'Save' and also a tooltip with text 'Save' is pointless.

I guess one option would be refactoring the Tooltip component to always mount the tooltip and switch is visibility with CSS. I'm not sure there are other ways, any thoughts and considerations are very welcome.

The main point here is to avoid a full re-rendering of the button (or other interactive control) the Tooltip it associated with.

[talldan]

Tooltip is a very quirky component, so I can see it might have this kind of issue. It might be good to put together a unit test case that replicates this and help prevent regressions when the problem is fixed (cc @ciampo @mirka who may be interested in this).

---

A full re-rendering is less than ideal for a couple reasons:

A render won't usually cause a focus loss. Even a full render of an app won't unless something purposely changes around the focused element when react has reconciled and is updating the DOM. A render is usually the term for calling a function component or calling the render function of a class component. This produces an updated state of the DOM, which is then reconciled against the current state of the DOM. Only dramatic differences should cause a DOM element to be replaced, but there are also some other practices that can cause issues.

I feel it's worth mentioning this as there's often an impression that rendering in React is bad. It may be bad for performance if it happens too often, but shouldn't really cause other issues unless there's buggy code in a component.

There's some info here on reconciliation - https://reactjs.org/docs/reconciliation.html. I imagine what's happening in tooltip is something like that 'Elements Of Different Types' section.

[ciampo]

Thank you for the ping!

We are planning on rewriting Tooltip entirely, switching from our custom implementation to ariakit (see details here) — that should hopefully improve the aspect flagged above, too.

---

I second what @talldan wrote above — here's an example of a bug caused by reconciliation that we fixed recently #43508

[afercia]

@talldan thanks for your feedback.

A render won't usually cause a focus loss. Even a full render of an app won't unless something purposely changes around the focused element when react has reconciled and is updating the DOM.

Terminology: by 'full re-rendering' I didn't mean the React 'render'. That's React terminology. I did mean re-rendering in the browser (or re-painting, if you like) as in: an element gets removed and re-injected, triggering a full re-rendering of a portion of the page. I know that happens when React isn't able to reconcile.

You can actually test this Tooltip behavior on #43952
-Edit a draft post and make sure it's saved so that the Save button shows the text 'Saved'.

• Make any edit so that the button text changes to 'Save draft'.
• Select the Save button in the dev tools inspector.
• Right click and enable 'Break on > node removal'.
• Click the Save button.
• See the button gets removed from the DOM. This triggers a focus loss.

Animated GIF to better illustrate:

[fabiankaegy]

@afercia is this still an issue now that #48222 has been merged?

[afercia]

@fabiankaegy yes this appears to still be a problem.

To reproduce:

• Comment out this line and build:

  gutenberg/packages/editor/src/components/post-saved-state/index.js

  Line 161 in 3ebb723

  showTooltip

• Edit a draft post and make sure it's saved so that the Save button shows the text 'Saved'.
• Make any edit so that the button text changes to 'Save draft'.
• Select the Save button in the dev tools inspector.
• Right click and enable 'Break on > node removal'.
• Click the Save button.
• Observe the button gets removed from the DOM (and then re-added). This triggers a focus loss.

As I mentioned above, right now the only workaround is to force the tooltip to always be rendered by setting the showTooltip prop to true. That's not ideal because in some caes it would be great to render the tooltip conditionally.

[ciampo]

I'm not sure this is really something that Tooltip can be blamed for — I looked into it and came up with this tentative solution #56490

[ciampo]

Moving the conversation to issue #56522