Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$250] Chat - Emoji category selection buttons not working #48693

Closed
2 of 6 tasks
IuliiaHerets opened this issue Sep 6, 2024 · 33 comments
Closed
2 of 6 tasks

[$250] Chat - Emoji category selection buttons not working #48693

IuliiaHerets opened this issue Sep 6, 2024 · 33 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Sep 6, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com 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): rybkina+082024@gmail.com
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:

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6594734_1725578443077.RPReplay_Final1725578253.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021833174513558591946
  • Upwork Job ID: 1833174513558591946
  • Last Price Increase: 2024-09-16
  • Automatic offers:
    • allgandalf | Reviewer | 104018090
Issue OwnerCurrent Issue Owner: @allgandalf
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 6, 2024
Copy link

melvin-bot bot commented Sep 6, 2024

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
Copy link
Author

@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
Copy link
Contributor

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

Chat---Emoji-category-selection-buttons-not-working-.-Issue-48693-.-Expensify-App.mp4

What alternative solutions did you explore? (Optional)

@bernhardoj
Copy link
Contributor

bernhardoj commented Sep 7, 2024

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.

const scrollToHeader = (headerIndex: number) => {
const calculatedOffset = Math.floor(headerIndex / CONST.EMOJI_NUM_PER_ROW) * CONST.EMOJI_PICKER_HEADER_HEIGHT;
runOnUI(() => {
'worklet';
scrollTo(emojiListRef, 0, calculatedOffset, true);
})();

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.

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

const emojiListRef = useAnimatedRef<FlashList<EmojiUtils.EmojiPickerListItem>>();

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 melvin-bot bot added the Overdue label Sep 9, 2024
@alexpensify alexpensify added the External Added to denote the issue can be worked on by a contributor label Sep 9, 2024
@melvin-bot melvin-bot bot changed the title Chat - Emoji category selection buttons not working [$250] Chat - Emoji category selection buttons not working Sep 9, 2024
Copy link

melvin-bot bot commented Sep 9, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 9, 2024
Copy link

melvin-bot bot commented Sep 9, 2024

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

@alexpensify
Copy link
Contributor

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
Copy link
Contributor

I will review the proposals tomorrow

@alexpensify
Copy link
Contributor

@allgandalf, any update for these proposals? Thanks!

@melvin-bot melvin-bot bot added the Overdue label Sep 13, 2024
@allgandalf
Copy link
Contributor

Lets go with @bernhardoj proposal here

🎀👀🎀 C+ reviewed

@melvin-bot melvin-bot bot removed the Overdue label Sep 15, 2024
Copy link

melvin-bot bot commented Sep 15, 2024

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

@nyomanjyotisa
Copy link
Contributor

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

Copy link

melvin-bot bot commented Sep 16, 2024

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

@alexpensify
Copy link
Contributor

Waiting for the review here

@flodnv
Copy link
Contributor

flodnv commented Sep 17, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Sep 17, 2024
@allgandalf
Copy link
Contributor

allgandalf commented Sep 17, 2024

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;
Screenshot 2024-09-17 at 6 47 34 PM

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

@melvin-bot melvin-bot bot removed the Overdue label Sep 17, 2024
@bernhardoj
Copy link
Contributor

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

@flodnv
Copy link
Contributor

flodnv commented Sep 18, 2024

Okay, makes sense, thanks 👍

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 18, 2024
Copy link

melvin-bot bot commented Sep 18, 2024

📣 @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
Copy link
Contributor

@flodnv To confirm, which solution should I use?

@flodnv
Copy link
Contributor

flodnv commented Sep 19, 2024

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

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Sep 19, 2024
@bernhardoj
Copy link
Contributor

Got it. PR is ready

cc: @allgandalf

@allgandalf
Copy link
Contributor

PR Approved ✅ , waiting to get merged ♻️

@alexpensify
Copy link
Contributor

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

@alexpensify
Copy link
Contributor

Flagging that this one went into production today.

@allgandalf
Copy link
Contributor

This should be ready for payment on 4th

@alexpensify
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

alexpensify commented Oct 8, 2024

Payouts due: 10-04-2024

Upwork job is here.

@alexpensify
Copy link
Contributor

@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!

@allgandalf
Copy link
Contributor

I can confirm that i have received the payment, thanks @alexpensify 😄

@bernhardoj
Copy link
Contributor

Requested in ND.

@JmillsExpensify
Copy link

$250 approved for @bernhardoj

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

7 participants