[HOLD for payment 2023-11-06] Refactor EmojiPickerMenu component for web to avoid unnecessary rerenders

[mountiny]

Coming from here

Proposal

Refactor EmojiPickerMenu component for web to avoid unnecessary rerenders

Problem

Currently, the logic responsible for highlighting the emoji on hover is living in EmojiPickerMenu component, which contains a highlightedIndex state, keeping the index number of emoji that need to be highlighted. Since this index changes each time user hovers in/hovers out, it causes rerender of whole EmojiPickerMenu component (see video 1 - console is showing the amount of EmojiPickerMenu rerenders and the rendered components are highlighted in the browser).

Solution

To address this issue, the logic for highlighting should be moved to EmojiPickerMenuItem component to rerender only the hovered component. This will prevent the whole EmojiPickerMenu from rerendering (see video 2 - console is showing the amount of EmojiPickerMenu rerenders and the rendered components are highlighted in the browser).
Also attaching screenshots from the profiler - you can see that originally the whole picker menu is rerendering on each hover change, and after fixing it the only thing that gets rerendered is a single menu item.

Issue Owner

Current Issue Owner: @muttmuure

[melvin-bot]

Current assignee @muttmuure is eligible for the Bug assigner, not assigning anyone new.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[TMisiukiewicz]

I’m Tomasz from Callstack, I’d like to work on this job

[TMisiukiewicz]

Hi, after further investigation I noticed that hovering is not the only way of highlighting an emoji - there is also a possibility of navigating through the list with tab & arrows. This makes the proposed solution incorrect, as highlightedIndex is primarily used for the keyboard navigation. While EmojiPickerMenuItem doesn't directly contain this state, its's behavior is closely tied with it. Introducing separate logic for highlighting items on hover would not be a good fit here, as it would break switching between mouse hover and using a keyboard navigation. In this case, looks like using a highlightedIndex state is reasonable and moving away from it would require a significant amount of effort and would likely complicate the component further.

Therefore, I wonder what the next steps should be?

[mountiny]

@TMisiukiewicz Is that how it has to work though? I dont think its common that when you Click Tab the focus on the next item then you had hovered over, hover is not focus.

[TMisiukiewicz]

@mountiny yeah, it's not very common but works that way now 🤷‍♂️ If there's no need of navigating from hovered emoji to it's closest elements by tab/arrows then it's doable. We can separate hover state as mentioned in the issue and then use highlightedIndex in the menu only to determine which element is currently focused. In that case we'll reduce rerenders of the menu when hovering, but keep in mind it will still happen when navigating with keyboard. I think if anyone use keyboard navigation after hover, we could just focus on the first item on the list. Let me know what you think