Make tabbable codeblocks if content overflows

[myasonik]

Summary

A fix for elastic/kibana#89490

Related to #3385 but doesn't address some of the bigger/broader questions and fixes one specific instance.

Checklist

• [ ] Check against all themes for compatibility in both light and dark modes
• [ ] Checked in mobile
• Checked in Chrome, Safari, Edge, and Firefox
• [ ] Props have proper autodocs and playground toggles
• [ ] Added documentation
• [ ] Checked Code Sandbox works for the any docs examples
• Added or updated jest tests
• Checked for breaking changes and labeled appropriately
• Checked for accessibility including keyboard-only and screenreader modes
• A changelog entry exists and is marked appropriately

[thompsongl]

Is this testable?

I haven't been able to get a test working. Seems like mutation observer callback isn't happening, even when doing manual component.setProps to trigger size or content changes.

[myasonik]

I haven't been able to get a test working. Seems like mutation observer callback isn't happening, even when doing manual component.setProps to trigger size or content changes.

Ditto - haven't been able to get anything meaningful working. I think this is an ok testing gap so I'll take the PR out draft mode once I make the code change but if others disagree+have ideas on how to address it, I'm happy to take another look.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_4463/

[cchaos]

Visually I think we need to consider what tab-able means as visual cues versus functional. For instance, do we need this visual indicator around an entire codeblock? We've run into the same issue with popovers #4443

[myasonik]

What should we see?

Because the area is scrollable, it should have a focus state (at the very least, when :focus-visible decides that one is necessary (though I don't love :focus-visible)).

The select probably has a focus state because it could be scrollable so browsers just assume it is and give it one. You could at least argue there that it doesn't need one because there are other ways to "scroll" through the options (i.e., you can arrow throw the options, you don't have to scroll the container).

This is in contrast to the recent discussion about Accordion content which we don't expect to be interactive in any way so it doesn't need a focus state at all.

Is what I see what you see?

Just to confirm that we're seeing the same stuff because I see a lot of discrepancy between browsers, themes, and input modality:

• Any browser with the legacy theme - I see no focus state no matter what
• In Chromium browsers with Amsterdam - I see the black border only when tabbing through the UI and no focus state when clicking into the code block
• In Safari with Amsterdam - clicking or tabbing into the code block gives me a black border
• In Firefox with Amsterdam - I see a blue border only when tabbing through the UI and no focus state when clicking into the code block

[cchaos]

Is what I see what you see?

Yes, this is mainly why I'm asking. Because there is an inconsistency between themes where the Amsterdam theme will show a focus ring when tabbing to the code block. If the default theme is ok in which we don't show any focus state at all, then I opt that we do the same for Amsterdam and specifically remove :focus-visible indicator.

[myasonik]

Sorry, it probably got lost in my previous message/I didn't explain very well.

Because this container needs to scroll, it's interactive. And because it's interactive, it needs a visible focus state.

So the default theme is incorrect but I was going to ignore that because Amsterdam is and Amsterdam is the future.

[myasonik]

Removing the :focus-visible pseudo-class would make the focus state visible whenever the user focused on the element (by tabbing to it, or by clicking on it).

Though I'd prefer that (both for simplicity's sake as well as someone who just prefers very clear focus indicators), I know there are other reasons that it might not be desired by many mouse users.

[cchaos]

Though I'd prefer that (both for simplicity's sake as well as someone who just prefers very clear focus indicators), I know there are other reasons that it might not be desired by many mouse users.

Yeah I consider this to be a very personal preference. Eventually we can get to the point where we can have more personal theme settings than just light vs dark mode. Something to consider in the move to Emotion.

---

So my main concern visually then is the fact that the focus ring doesn't encompass the controls. Just looks awkward. I pulled the PR down and tried to test using :focus-within and such, but that then artifacts down to any (mouse or keyboard) interaction into the code block whether or not the things scrolls. So I'll just make a different issue to follow up later on how to expand that focus ring.

[myasonik]

Eventually we can get to the point where we can have more personal theme settings than just light vs dark mode. Something to consider in the move to Emotion.

That'd be very cool.

So I'll just make a different issue to follow up later on how to expand that focus ring.

@cchaos So are you ok merging this as is then? (Will still wait for a technical review from @thompsongl or @chandlerprall too, if so)

[cchaos]

This is fine from a design standpoint. 👍

[thompsongl]

Overall LGTM. Just the one question.

Also, you'll want to merge/rebase to get the changelog entry in the right spot.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_4463/

[thompsongl]

Thanks for making the update!
LGTM!