Wildcard: Menu button component has bad performance and glitching

[vovakulikov]

Background

On the dashboard code insights page we render a lot of insight cards, each card renders a menu button component within the card. This means we render a lot of menu buttons at the same time and since this PR we always render the menu button content elements even if they're not visible (when the menu button isn't open). This increases the number of elements on the page and therefore increases the paint load for the browser.

Screen.Recording.2022-07-05.at.16.30.05.mov

Proposal

We should not render elements that are not visible on the page. As I recall the original fix about rendering elements on the page in order to focus the first available item came from the fact that overwise reach UI does not focus the first available item. I think this is a good sign for us that we should try different components for menu button pattern that doesn't require rendering on the page all time.

[sourcegraph-bot-2]

Heads up @taylorsperry @jasongornall - the "team/frontend-platform" label was applied to this issue.

[umpox]

(Copied from Slack): https://sourcegraph.slack.com/archives/C01LTKUHRL3/p1657100075260849?thread_ts=1657053300.433479&cid=C01LTKUHRL3

It seems this is primarily because we run the render / positioning logic on these elements.

This change fixes it, I think. Maybe could even do it earlier before render is even called so we don’t repeat this logic every event loop

The question is just, do we need to be running positioning logic on hidden elements? I wonder if this could cause future perf issues elsewhere

@vovakulikov LMK what you think, maybe I’ve missed something obvious - happy to revert if we can’t get a nice solution here

[vovakulikov]

@umpox I also was thinking about this, but the problem remains even if we have an early return here. I think we should investigate more in this more to have a proper solution, maybe even change the API of tether logic.

But to be honest I could catch this bug only in Safari. And freezes that I could see I saw them only in Safari as well, so we probably can go without reverting a11y PR about the menu here if the frontend platform team has time to fix it in the near future. I can help with that of course, I think you are right about not running position logic if the target element is hidden. I also think that mutations that we're doing inside of tether over target and popover elements might be a reason for glitching in safari.

Screen.Recording.2022-07-06.at.11.06.53.mov