Chat - Emoji category selection buttons not working

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.30-4 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4935849 Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause Internal Team

Action Performed:

1. Launch App
2. Tap on any chat
3. Open emoji picker near compose and tap on any icon to select section under search field

Expected Result:

Emoji picker should be scroll to selected section.

Actual Result:

Scroll emoji via icon doesn't works. Nothing happens when tap on any icon to select section under search field

Workaround:

Unknown

Platforms:

• [x] Android: Native
• [ ] Android: mWeb Chrome
• [x] iOS: Native
• [ ] iOS: mWeb Safari
• [ ] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/bab87e7f-5d2d-43b5-8795-a65df1809535

View all open jobs on GitHub

[melvin-bot[bot]]

Triggered auto assignment to @alexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[IuliiaHerets]

@alexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[nyomanjyotisa]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Emoji category selection buttons not working

What is the root cause of that problem?

The scrollToHeader here on native not working properly

What changes do you think we should make in order to solve the problem?

Change scrollToHeader here to the following

    const scrollToHeader = useCallback(
        (headerIndex: number) => {
            if (!emojiListRef.current) {
                return;
            }
    
            const calculatedOffset = Math.floor(headerIndex / CONST.EMOJI_NUM_PER_ROW) * CONST.EMOJI_PICKER_HEADER_HEIGHT;
    
            emojiListRef.current.scrollToOffset({
                offset: calculatedOffset,
                animated: true,
            });
        },
        [emojiListRef],
    );

RESULT

https://github.com/user-attachments/assets/bb7bbee5-ccdf-4384-9c58-67131f4c0530

What alternative solutions did you explore? (Optional)

[bernhardoj]

Edited by proposal-police: This proposal was edited at 2024-09-07 11:26:48 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Emoji category button doesn't work.

What is the root cause of that problem?

When we press the emoji category, we call scrollTo and pass the ref to the function. https://github.com/Expensify/App/blob/4af620b439636f714ff72a26320ed57f9d2faac3/src/components/EmojiPicker/EmojiPickerMenu/index.native.tsx#L68-L74

scrollTo is a reanimated function to scroll synchronously which requires an animated component or RN built-in component.

However, we are using FlashList as the list component. https://github.com/Expensify/App/blob/4af620b439636f714ff72a26320ed57f9d2faac3/src/components/EmojiPicker/EmojiPickerMenu/BaseEmojiPickerMenu.tsx#L105-L106

But the doc also mentioned that it would work if the component implements getNativeScrollRef for the new arch which doesn't exist on FlashList and they have no plan on adding it.

This previously worked fine because we have a patch to add the getNativeScrollRef to flash-list and recylcerlistview which is the deps of flash-list, but was removed when upgrading RN. (the linked PR is part of the RN upgrade)

Btw, we used an animated FlatList when the scroll code was introduced.

What changes do you think we should make in order to solve the problem?

Use the ref scrollToOffset directly.

emojiListRef.current?.scrollToOffset({offset: calculatedOffset, animated: true})

Change it on both index and index.native files. Then, we can change this to useRef https://github.com/Expensify/App/blob/4af620b439636f714ff72a26320ed57f9d2faac3/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.ts#L13

What alternative solutions did you explore? (Optional)

There is already an upstream PR that will add the function but it has been stale for months. So, we can re-add the patch back that adds the getNativeScrollRef, but only the patch for the flash list code.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @allgandalf (External)

[alexpensify]

I'm able to replicate this experience on my Android device. @allgandalf, please review the proposals and identify if one will fix this issue. Thanks!

[allgandalf]

I will review the proposals tomorrow

[alexpensify]

@allgandalf, any update for these proposals? Thanks!

[allgandalf]

Lets go with @bernhardoj proposal here

🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

Triggered auto assignment to @flodnv, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[nyomanjyotisa]

@allgandalf How about my proposal? I think it basically the same with the selected proposal

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[alexpensify]

Waiting for the review here

[flodnv]

@allgandalf could you please clarify why you did not choose @nyomanjyotisa's proposal?

[allgandalf]

yeah, In general my approach to selecting a proposal is correct and clear RCA (i.e. we should know why the bug occured in the first place) and then moving to solution, I found that @bernhardoj has a clear RCA than @nyomanjyotisa, and hence i preferred their solution.

In Our proposal guidelines we have;

But if we want to choose @nyomanjyotisa 's proposal here, i have no problem with that!

[bernhardoj]

Just FYI, I also write the alternative if we want to keep using the animated ref.

[flodnv]

Okay, makes sense, thanks 👍

[melvin-bot[bot]]

📣 @allgandalf 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

[bernhardoj]

@flodnv To confirm, which solution should I use?

[flodnv]

I feel like the main proposed solution (not the alternative) is best in regards to ensuring this regression does not come back?

[bernhardoj]

Got it. PR is ready

cc: @allgandalf

[allgandalf]

PR Approved ✅ , waiting to get merged ♻️

[alexpensify]

Awesome, now we wait for this one to go to Prouduction.

[alexpensify]

Flagging that this one went into production today.

[allgandalf]

This should be ready for payment on 4th

[alexpensify]

Something weird is going on with Upwork, I'm going to hold to confirm if there is an issue with the other payments I just sent out.

[alexpensify]

There is an issue an Upwork that is being investigated to address the payment process. I'm going to hold until there is a fix. Convo: https://expensify.slack.com/archives/C01GTK53T8Q/p1728319262830679

[alexpensify]

Payouts due: 10-04-2024

• [x] Contributor: $250 @allgandalf
• [x] Contributor+: $250 @bernhardoj

Upwork job is here.

[alexpensify]

@allgandalf - our team figured out the Upwork bug, and I tried to complete the payment via another payment path in Upwork. Their support team is working on a permanent fix for the bug. I'd really appreciate it if you could confirm today's payment was sent successfully. Thanks!

---

@bernhardoj please submit a request in Chat!