[$500] [MEDIUM] Migrate the EmojiPickerMenu to Flashlist

[mountiny]

Part of #28902

Let's migrate EmojiPickerMenu to use Flashlist component instead of FlatList

Callstack is working on the migration right now.

Lets use this issue for daily updates on this migration

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01717c0ee29a49c13a
• Upwork Job ID: 1757786794067501056
• Last Price Increase: 2024-02-14

[melvin-bot]

Triggered auto assignment to @sophiepintoraetz (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[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]

Hi, I'm Tomasz from Callstack, I'd like to work on that issue

[roryabraham]

@TMisiukiewicz do you expect to make daily progress on this issue?

[TMisiukiewicz]

@roryabraham yes, this issue will be my main focus starting from tomorrow 👍

[esh-g]

Related bug here: https://expensify.slack.com/archives/C01GTK53T8Q/p1699208483024949

[TMisiukiewicz]

Started working on migrating the EmojiPickerMenu for web.

[TMisiukiewicz]

No progress today, as I had to work on some regressions and i was investigating failing tests in LHNOptionList migration. I'm off Thursday and Friday, in case there is a need of pushing this forward before Monday, I know @vitHoracek already pinged CK team if anyone could grab this

[shubham1206agra]

I think this issue is related #30948

[melvin-bot]

@TMisiukiewicz, @sophiepintoraetz, @roryabraham Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

@TMisiukiewicz, @sophiepintoraetz, @roryabraham Huh... This is 4 days overdue. Who can take care of this?

[TMisiukiewicz]

Back to work on this task today and continued with migration of the web component. Working on adopting the styles to the list correctly with FlashList, since it does not support e.g. contentContainerStyle properties other than background and padding.

Regarding the issue mentioned above, it proposes using updateCellsBatchingPeriod prop, which is unavailable in FlashList. I'd suggest implementing FlashList first, and then, if the issue still exists, fixing it with the solution available in FlashList.

[TMisiukiewicz]

Still in progress, my initial idea was to create separate PRs for web and mobile to ship first part of my work faster, but today I found out some changes in EmojiPickerMenuItem styles need to be done, so it affects all platforms.

[TMisiukiewicz]

Some minor CSS updates and fixing scroll to index left for web, then I'll take care of mobile version

[TMisiukiewicz]

Some minor things to be re-checked, I should create a PR tomorrow.

[melvin-bot]

This issue has not been updated in over 15 days. @TMisiukiewicz, @sophiepintoraetz, @roryabraham eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[muttmuure]

Great!

[melvin-bot]

Triggered auto assignment to @alexpensify (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

[alexpensify]

Catching up here, looks like we are moving forward.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[roryabraham]

The minor regression was not treated as a blocker, and has already been fixed.

[roryabraham]

Hey @TMisiukiewicz 👋🏼

I think there's still more we can do to DRY up the implementation between web and native. I know appreciate you helping us work through all this technical debt. A few suggestions/observations:

• On web we're using throttle for filterEmojis, on native we're using debounce. It seems like it should be safe to just use debounce on both platforms. Let's try setting the timeout to 200ms (which should be fast enough that you don't really notice a delay), but long enough that we're not doing a ton of extra work. In the long run (after we get the new arch + concurrent react set up), I think we'll probably want to replace most of these usages of debounce + setState with startTransition
• It looks like there are some unnecessary differences between web and mobile around emojiListRef / scrollToHeader. I think that both of those can probably be moved to the base implementation
• If we create a useMouseMoveHandler hook that's a no-op on mobile (at least, for now) then we can DRY up the mouseMoveHandler logic and move that to the base implementation.

There might be more, but these seem like low-hanging fruit.

[TMisiukiewicz]

hey @roryabraham! My hands are full right now with some stuff with high priority, but I can get back to this once I'm done with it. Thanks for pointing out the parts to focus on, if I have any other observations what could be potentially DRYed up, I'll post them here as well 👍

[roryabraham]

Ok, sounds good. I think this can be treated as kind of lower priority for now as it doesn't really affect users and is more about code quality

[jjcoffee]

@alexpensify Would you be able to assign me to the issue as I reviewed the PR? Thanks!

[alexpensify]

Yep, done!

[alexpensify]

Weekly Update: Making progress here

[jjcoffee]

@alexpensify I think the payment for PR review on this is due as the PR hit production last week. 🙇 Not sure why the automation failed!

[alexpensify]

@jjcoffee - thanks for flagging, I'll work on the next steps since automation didn't kick in here.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01717c0ee29a49c13a

[melvin-bot]

Current assignee @jjcoffee is eligible for the External assigner, not assigning anyone new.

[alexpensify]

@jjcoffee please accept the offer in Upwork - https://www.upwork.com/jobs/~01717c0ee29a49c13a

Thanks!

[jjcoffee]

@alexpensify Accepted, thanks!

[alexpensify]

Here is the payment summary:

• External issue reporter - N/A
• Contributor that fixed the issue - @TMisiukiewicz is from an agency-contributor and not due payment
• Contributor+ that helped on the issue and/or PR - @jjcoffee paid $500

Upwork Job: https://www.upwork.com/jobs/~01717c0ee29a49c13a

Extra Notes regarding payment: @jjcoffee has been paid via Upwork.