[HOLD for payment 2024-06-06] [$500] iOS - Chat - Unable to select emoji if searching for specific one

[lanitochka17]

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: 1.4.73-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
**Issue reported by:**Applause - Internal Team

Action Performed:

1. Open the app and log in
2. Open any report
3. Tap the emoji picker
4. Enter a search word that would return only one result and not in recently used, e.g. panda, fox, pizza etc
5. Select the emoji

Expected Result:

User is able to select the emoji, the emoji is displayed in compose

Actual Result:

User is unable to select the emoji, nothing happens after tapping the emoji

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6479977_1715638337583.IMG_6865.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ff89e96c3fe3cbbb
• Upwork Job ID: 1790382115707867136
• Last Price Increase: 2024-06-07
• Automatic offers:
• hungvu193 | Reviewer | 0
• QichenZhu | Contributor | 0

Issue Owner

Current Issue Owner: @garrettmknight

[melvin-bot]

Triggered auto assignment to @garrettmknight (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.

[lanitochka17]

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

[lanitochka17]

We think that this bug might be related to #vip-vsp

[melvin-bot]

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

[melvin-bot]

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

[garrettmknight]

Reproduced, opening up.

[QichenZhu]

Proposal

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

Unable to select the only emoji in the emoji picker.

What is the root cause of that problem?

The FlashList's footer overlaps the emoji because the footerDiff method counts subviews incorrectly.

App/patches/@shopify+flash-list+1.6.3.patch

Lines 1012 to 1015 in 153021e

	+ } else if viewsToLayout.count == 1 {
	+ let firstChild = viewsToLayout[0]
	lastMaxBoundOverall = horizontal ? firstChild.frame.maxX : firstChild.frame.maxY
	}

Specifically, when there is only one emoji, it counts as 2. The reason is explained in the fixLayout method:

App/patches/@shopify+flash-list+1.6.3.patch

Lines 968 to 971 in 153021e

	+ // On Fabric, due to view flattening children of AutoLayoutView are moved one level up, so they appear
	+ // as children of AutoLayoutViewComponentView in view hierarchy. viewsToLayout property takes it under
	+ // consideration, returning children of AutoLayoutViewComponentView when on Fabric. Because of that
	+ // AutoLayoutView may be on the list, in which case we want to ignore it.

To confirm, you can add a visible footer to the FlashList, and you will find the footer misplaced.

<FlashList
    ...
    ListFooterComponent={<Text>Footer</Text>}
    ListFooterComponentStyle={{backgroundColor: '#ff00ff20'}}
    ...

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

1. Modifing the local file node_modules/@shopify/flash-list/ios/Sources/AutoLayoutView.swift as below, in addition to the current patch.

@@ -134,6 +134,10 @@
         var maxBoundNextCell: CGFloat = 0
         let correctedScrollOffset = scrollOffset - (horizontal ? frame.minX : frame.minY)
         lastMaxBoundOverall = 0
+        if cellContainers.count == 1 {
+            let firstCellContainer = cellContainers[0]
+            lastMaxBoundOverall = horizontal ? firstCellContainer.frame.maxX : firstCellContainer.frame.maxY
+        }
         cellContainers.indices.dropLast().forEach { index in
             let cellContainer = cellContainers[index]
             let cellTop = cellContainer.frame.minY
@@ -282,12 +286,6 @@
     }
 
     private func footerDiff() -> CGFloat {
-        if viewsToLayout.count == 0 {
-            lastMaxBoundOverall = 0
-        } else if viewsToLayout.count == 1 {
-            let firstChild = viewsToLayout[0]
-            lastMaxBoundOverall = horizontal ? firstChild.frame.maxX : firstChild.frame.maxY
-        }
         let autoLayoutEnd = horizontal ? frame.width : frame.height
         return lastMaxBoundOverall - autoLayoutEnd
     }

2. Updating the patch using patch-package to persist these changes.

What alternative solutions did you explore? (Optional)

Workaround: Adding ListFooterComponentStyle={{display: 'none'}} to the FlashList.

[hungvu193]

Thanks @QichenZhu, your proposal makes sense to me, can you please provide more detail about your main solution so I can test it?
AFAIK, we apply that patch to enable new arch, do you think with your change (in your main solution) it still works?

[QichenZhu]

Thanks @hungvu193. For testing, I modified the local file node_modules/@shopify/flash-list/ios/Sources/AutoLayoutView.swift as below, in addition to the current patch.

@@ -134,6 +134,10 @@
         var maxBoundNextCell: CGFloat = 0
         let correctedScrollOffset = scrollOffset - (horizontal ? frame.minX : frame.minY)
         lastMaxBoundOverall = 0
+        if cellContainers.count == 1 {
+            let firstCellContainer = cellContainers[0]
+            lastMaxBoundOverall = horizontal ? firstCellContainer.frame.maxX : firstCellContainer.frame.maxY
+        }
         cellContainers.indices.dropLast().forEach { index in
             let cellContainer = cellContainers[index]
             let cellTop = cellContainer.frame.minY
@@ -282,12 +286,6 @@
     }
 
     private func footerDiff() -> CGFloat {
-        if viewsToLayout.count == 0 {
-            lastMaxBoundOverall = 0
-        } else if viewsToLayout.count == 1 {
-            let firstChild = viewsToLayout[0]
-            lastMaxBoundOverall = horizontal ? firstChild.frame.maxX : firstChild.frame.maxY
-        }
         let autoLayoutEnd = horizontal ? frame.width : frame.height
         return lastMaxBoundOverall - autoLayoutEnd
     }

Regarding your second question, I believe so because these modifications are made to the patched local file and then I'll update the patch to persist these changes.

[hungvu193]

Thanks for your updates here. Let me test it carefully since it will affect lot of things.

[hungvu193]

It's working well! Can you update your proposal to explain your change here? Thank you 😄

Screen.Recording.2024-05-17.at.18.01.45.mov

[QichenZhu]

@hungvu193 Thank you for your time!

[QichenZhu]

Proposal

Updated

[hungvu193]

@QichenZhu 's proposal here looks good to me! 🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

@garrettmknight, @hungvu193, @neil-marcellini Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[neil-marcellini]

@QichenZhu 's proposal here looks good to me! 🎀 👀 🎀 C+ reviewed

Ok, looks good to me. What's the process to get the fix merged into the actual repo eventually?

[garrettmknight]

Got to staging yesterday, will keep an eye on it in case the automation doesn't run.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.77-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix: Unable to select emoji if searching for specific one #42407

If no regressions arise, payment will be issued on 2024-06-06. 🎊

For reference, here are some details about the assignees on this issue:

• @hungvu193 requires payment automatic offer (Reviewer)
• @QichenZhu requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@hungvu193] The PR that introduced the bug has been identified. Link to the PR:
• [@hungvu193] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@hungvu193] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@hungvu193] Determine if we should create a regression test for this bug.
• [@hungvu193] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@garrettmknight] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[QichenZhu]

As announced in Slack, could the bounty be increased since the priority is high? Thank you!

[hungvu193]

• The PR that introduced the bug has been identified. Link to the PR: Enable Fabric and TurboModules on iOS & Android #13767
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/13767/files#r1627535636
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
• Determine if we should create a regression test for this bug.
• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression test:

1. Open the app and log in.
2. Open any report.
3. Tap the emoji picker.
4. Enter a search word that would return only one result and not in recently used, e.g. panda, fox, pizza etc
5. Select the emoji.
6. Verify that emoji is selected.

Do we 👍 or 👎 ?

[hungvu193]

As announced in Slack, could the bounty be increased since the priority is high? Thank you!

I think this is complex issue and I was about to request a raise the bounty right before you posted your proposal.

[garrettmknight]

There have been questions about what constitutes a job being auto-increased to $500. These jobs are only the ones that have the High Priority label added to them (ie. issue with HIGH or CRITICAL in the title or project status/priority are not auto-increased to $500).

To be clear, this wouldn't be a high priority issue as described in the slack post.

[garrettmknight]

@hungvu193 can you help me understand what about this issue was particularly complex? I'm not doubting you, just trying to understand.

[hungvu193]

@hungvu193 can you help me understand what about this issue was particularly complex? I'm not doubting you, just trying to understand.

Yeah. So this issue needs to be fixed on the native side (Swift code). It will take more time to debug and investigate because it will require you to understand the native code as well. In fact, this issue has been opened for the last 3 weeks with no proposals until @QichenZhu posted his proposal.

[garrettmknight]

In fact, this issue has been opened for the last 3 weeks with no proposals until @QichenZhu posted his proposal.

I don't think this is the case - it looks like he put his proposal forward 1 day after I opened this up for proposals. It did take a few weeks to get through, but I don't think that's indicative of complexity with the merge freeze in mind.

[hungvu193]

Actually when an issue was created, contributors will post the proposals even when Help Wanted label isn't added.
But yeah, I agreed with you at this point 😄

it looks like he put his proposal forward 1 day after I opened this up for proposals

[mallenexpensify]

As announced in Slack, could the bounty be increased since the priority is high? Thank you!

@QichenZhu (and @hungvu193 ), unfortunately no, I commented on the link above earlier this week with

There have been questions about what constitutes a job being auto-increased to $500. These jobs are only the ones that have the High Priority label added to them (ie. issue with HIGH or CRITICAL in the title or project status/priority are not auto-increased to $500).

If you believe the price should be increased because of added scope of the job, ie. I think @QichenZhu will need to raise an issue in the upstream repo, please comment with detailed reasoning about the added scope and the requested increase in price (either @QichenZhu or @hungvu193 can do this)

[QichenZhu]

@mallenexpensify @garrettmknight Thank you for clarifying the differences between title/label/status. I understand that the priority is decided solely on your end.

Regarding the scope of work, @hungvu193 thoroughly tested my proposal as it modified a basic component and might affect a lot of things. He also explored the upstream repo and identified a related upstream PR.

[garrettmknight]

Hey @QichenZhu @hungvu193 - thanks for the detail. After reviewing, I'd agree this deserves an increased bounty!

[melvin-bot]

Upwork job price has been updated to $500

[garrettmknight]

Payment Summary:

• Contributor: @QichenZhu $500 paid via upwork
• Reviewer: @hungvu193 $500 paid via upwork

[QichenZhu]

Received. Thank you for the bonus!