[EuiInnerText] Returns empty string inside EuiPopover

[thompsongl]

EuiInnerText returns an empty string for innerText (node contents) when used within EuiPopover.
The ref is correctly set and the node does in fact have content, but node.innerText returns empty. node.textContent, however, correctly returns the content string. Given

eui/src/components/inner_text/inner_text.tsx

Lines 29 to 36 in 3d9a593

	// Check for `innerText` implementation rather than a simple OR check
	// because in real cases the result of `innerText` could correctly be `null`
	// while the result of `textContent` could correctly be non-`null` due to
	// differing reliance on browser layout calculations.
	// We prefer the result of `innerText`, if available.
	'innerText' in node
	? node.innerText
	: node.textContent || innerTextFallback

we accept the empty string over the content string.

There are important differences between the two methods, most notably that

innerText only shows "human-readable" elements

and

innerText is aware of styling and won’t return the text of “hidden” elements.

It's likely that while EuiPopover is opening (and at the time the node is evaluated) some style or attribute is marking the node as hidden or not human-readable. innerText has the correct content inside EuiPopover if delayed until after the popover is stable open.

Repro: https://codesandbox.io/embed/cold-currying-tpfgf?fontsize=14&hidenavigation=1&theme=dark

[thompsongl]

Tracked it down to (the somewhat obvious) visibility: hidden thing we do prior to stable open:

eui/src/components/popover/_popover.scss

Line 41 in 3d9a593

visibility: hidden; /* 2 */

Unsure how to resolve yet.

[cee-chen]

Unlike opacity, visibility can be set separately in child elements and override the parent element. Check it out:

https://codesandbox.io/s/7dhmm?file=/index.html (may need to refresh the codesandbox iframe or view in new window, JS is being finicky)

This means even though we set opacity: 0; visibility: hidden on the parent .euiPopover__panel, we can immediately set a wrapper inside it with visibility: visible and innerText will work again.

Alternatively, another point to consider: do we need the visibility property in the first place? Or will just opacity suffice in terms of animating the popover in?

[thompsongl]

do we need the visibility property in the first place? Or will just opacity suffice in terms of animating the popover in?

This is where I went, also.
We do look for visibility: hidden, but I don't see any reason we couldn't look for opacity: 0 instead:

eui/src/components/popover/popover.tsx

Lines 436 to 439 in 3d9a593

	// #1 is the whole panel hidden? If so, schedule another check
	// #2 panel is visible but no tabbables exist, move focus to the panel
	const panelVisibility = window.getComputedStyle(this.panel).visibility;
	if (panelVisibility === 'hidden') {

I'm going to try your other suggestion, also

[cee-chen]

++, if there isn't a specific reason we're animating/toggling visibility (accessibility or similar), I think it makes the most sense to animate only opacity.

[thompsongl]

a11y is the only thing I can think of, because visibility affects screen readers and tab order whereas opacity does not.

[cee-chen]

Right, but arguably that's better experience that screen readers get to focus immediately into the popover even though they can't see it - does a visual fade-in animation really need to delay non or low-sighted users?

[cee-chen]

I'm testing with VoiceOver now in local vs production (specifically data grids and forms in popovers) and not seeing a noticeable issue with removing visibility from the fade in animation. I'm still even seeing a slight delay in the screen reader focus moving into the new popover. So at least on that end I think we're still good.