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

[HOLD for #16078][$4000] The emoji recommendation doesn’t show up on android on going back from the emoji picker unlike Mweb chrome #16364

Closed
1 of 6 tasks
kavimuru opened this issue Mar 21, 2023 · 63 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 Monthly KSv2

Comments

@kavimuru
Copy link

kavimuru commented Mar 21, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Open staging dot on chrome on mweb
  2. Go to any chat and type in :person to get the recommendation list. Now immediately click on the emoji icon on the chat box to open up the emoji picker (Do not pick any)
  3. Next click somewhere on the screen above the emoji picker to go back
  4. Notice that the keyboard is focused and the emoji recommendation still appears
  5. But now follow the same process on android and see that the emoji recommendation list doesn’t appear even though the keyboard is still focused

Expected Result:

Emoji recommendation list should appear on android like that of the mweb chrome to maintain consistency

Actual Result:

Emoji recommendation list doesn’t appear on android when coming back from the emoji picker and is inconsistent with the mweb chrome

Workaround:

unknown

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.2.88-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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

emoji.mp4
az_recorder_20230321_181359.1.mp4

Expensify/Expensify Issue URL:
Issue reported by: @priya-zha
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1679395061542339

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~019708d74295d1af77
  • Upwork Job ID: 1640189765954387968
  • Last Price Increase: 2023-06-01
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 21, 2023
@MelvinBot
Copy link

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

@MelvinBot
Copy link

MelvinBot commented Mar 21, 2023

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

@kadiealexander
Copy link
Contributor

Can't reproduce in TestFlight, have asked @jliexpensify to test on a physical android device for me.

@jliexpensify
Copy link
Contributor

jliexpensify commented Mar 22, 2023

Ok I can repro in v88 (Google Pixel 3a), the same as @kavimuru 's 2nd video above:

image

But I wonder if this is just an Android "feature"? Because I just tried the same process in Slack and I get the same behaviour.

@kadiealexander
Copy link
Contributor

kadiealexander commented Mar 22, 2023

Thanks Jli!! I've raised your flag about other native apps having the same behaviour here.

@kadiealexander kadiealexander added the External Added to denote the issue can be worked on by a contributor label Mar 27, 2023
@melvin-bot melvin-bot bot changed the title The emoji recommendation doesn’t show up on android on going back from the emoji picker unlike Mweb chrome [$1000] The emoji recommendation doesn’t show up on android on going back from the emoji picker unlike Mweb chrome Mar 27, 2023
@MelvinBot
Copy link

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

@MelvinBot
Copy link

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

@MelvinBot
Copy link

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 27, 2023
@MelvinBot
Copy link

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

@kadiealexander
Copy link
Contributor

Not overdue.

@melvin-bot melvin-bot bot removed the Overdue label Mar 27, 2023
@bernhardoj
Copy link
Contributor

Proposal

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

There is inconsistency behavior of emoji suggestion between native and web. On native, after closing the emoji picker, the emoji suggestion does not appear even though we have typed the emoji code previously on the composer.

What is the root cause of that problem?

The real issue here is that, when we focus the composer with emoji code (for example :sm) on native, the suggestion never appears. If we fix that, this inconsistency issue will be solved too. When we close the emoji picker, the main composer will take the focus and should show the suggestion, but because the issue I mentioned above, the emoji suggestion never appears. Why?

Currently, we use onSelectionChange to decide whether we should show the emoji suggestion or not.

onSelectionChange(e) {
this.setState({selection: e.nativeEvent.selection});
this.calculateEmojiSuggestion();
}

calculateEmojiSuggestion() {
const leftString = this.state.value.substring(0, this.state.selection.end);
const colonIndex = leftString.lastIndexOf(':');
const isCurrentlyShowingEmojiSuggestion = this.isEmojiCode(this.state.value, this.state.selection.end);
// the larger composerHeight the less space for EmojiPicker, Pixel 2 has pretty small screen and this value equal 5.3
const hasEnoughSpaceForLargeSuggestion = this.props.windowHeight / this.state.composerHeight >= 6.8;
const isEmojiPickerLarge = !this.props.isSmallScreenWidth || (this.props.isSmallScreenWidth && hasEnoughSpaceForLargeSuggestion);
const nextState = {
suggestedEmojis: [],
highlightedEmojiIndex: 0,
colonIndex,
shouldShowSuggestionMenu: false,
isEmojiPickerLarge,
};
const newSuggestedEmojis = EmojiUtils.suggestEmojis(leftString);
if (newSuggestedEmojis.length && isCurrentlyShowingEmojiSuggestion) {
nextState.suggestedEmojis = newSuggestedEmojis;
nextState.shouldShowSuggestionMenu = !_.isEmpty(newSuggestedEmojis);
}
LayoutAnimation.configureNext(LayoutAnimation.create(50, LayoutAnimation.Types.easeInEaseOut, LayoutAnimation.Properties.opacity));
this.setState(nextState);
}

However, if the selection is the same, the onSelectionChange won't be called on native. For example, if I type :sm, the selection start and end is at 3. Then I open emoji picker (this will blur the composer) and close it. The composer then takes the focus back with the selection position (cursor) is the same when we blur it so the onSelectionChange is not called. However, this behavior is different on web. On web, it will always call onSelectionChange even though the selection is the same. We can easily reproduce it by just creating a simple text input.

TextInput.React.Native.-.Personal.-.Microsoft.Edge.2023-03-27.13-06-25.mp4

You can see at the video above that every time we click the text input with the same selection, it always calls the onSelectionChange.

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

There are 2 options that depends on what behavior (for all platform) we want.

  1. If we want that every time we refocus the composer (this includes that when we close the emoji picker), the emoji suggestion appears, then we should call calculateEmojiSuggestion on text input focus (call it inside setIsFocused if shouldHighlight is true).
    setIsFocused(shouldHighlight) {
    this.setState({isFocused: shouldHighlight});
    }
  2. Otherwise, inside onSelectionChange function, we only call calculateEmojiSuggestion if the new selection is different from current selection. (this means that if we close the emoji picker, all platform won't show the suggestion)

Or maybe we can apply both?

@kaushiktd
Copy link
Contributor

kaushiktd commented Mar 27, 2023

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

The emoji recommendation doesn’t show up on android on going back from the emoji picker unlike Mweb chrome.

What is the root cause of that problem?

when user focus on composer it will call setIsFocused() but when app state changes(like keyboard or and close - active to inactive), it will call onBlur method which called
this.resetSuggestedEmojis();
to clear all the suggested emojis, this is the main root cause of this issue.

this.resetSuggestedEmojis();

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

when the app state changes, it will first focus on the composer then calculateEmojiSuggestion() is called to get suggested emojis based on modal picker is open or not and composer has emojis code.
so you need to call
this.calculateEmojiSuggestion()
in resetSuggestedEmojis() on the below-mentioned file:

/**
     * Clean data related to EmojiSuggestions
     */
    resetSuggestedEmojis() {
        this.setState({
            suggestedEmojis: [],
            shouldShowSuggestionMenu: false,
        }, () => {
            this.calculateEmojiSuggestion()
        });
    }

@Santhosh-Sellavel Santhosh-Sellavel removed their assignment Mar 27, 2023
@Santhosh-Sellavel
Copy link
Collaborator

@chiragsalian or @kadiealexander Can you assign another C+ here, I've unassigned here as I can't get to this sooner thanks!

@chiragsalian chiragsalian added External Added to denote the issue can be worked on by a contributor and removed External Added to denote the issue can be worked on by a contributor labels Mar 28, 2023
@MelvinBot
Copy link

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

@MelvinBot
Copy link

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

@kadiealexander
Copy link
Contributor

Still on hold.

@melvin-bot melvin-bot bot removed the Overdue label Jul 28, 2023
@melvin-bot melvin-bot bot added the Overdue label Aug 7, 2023
@kadiealexander
Copy link
Contributor

Still on hold.

@melvin-bot melvin-bot bot removed the Overdue label Aug 8, 2023
@melvin-bot melvin-bot bot added the Overdue label Aug 21, 2023
@kadiealexander
Copy link
Contributor

Still on hold, shifting to monthly.

@melvin-bot melvin-bot bot removed the Overdue label Aug 22, 2023
@kadiealexander kadiealexander added Monthly KSv2 and removed Weekly KSv2 labels Aug 22, 2023
@melvin-bot melvin-bot bot added the Overdue label Sep 22, 2023
@kadiealexander
Copy link
Contributor

Still on hold

@melvin-bot melvin-bot bot removed the Overdue label Sep 25, 2023
@melvin-bot melvin-bot bot added the Overdue label Oct 26, 2023
@kadiealexander
Copy link
Contributor

Still on hold

@melvin-bot melvin-bot bot removed the Overdue label Oct 27, 2023
@melvin-bot melvin-bot bot added the Overdue label Nov 27, 2023
@0xmiros
Copy link
Contributor

0xmiros commented Nov 27, 2023

Not overdue

@melvin-bot melvin-bot bot removed the Overdue label Nov 27, 2023
@quinthar
Copy link
Contributor

I just tested this and can't reproduce on a Pixel 5. Can anyone still reproduce?

@kaushiktd
Copy link
Contributor

I've tried to change to the gradle version or react-native-vision-camera version also with possible solutions. But still getting the same error message.
Screenshot 2023-12-26 at 2 25 24 PM
unnamed
Screenshot 2023-12-26 at 2 26 04 PM

@melvin-bot melvin-bot bot added the Overdue label Dec 29, 2023
@kadiealexander
Copy link
Contributor

Sorry, back from OOO! Have asked an android using pal (@jliexpensify) to try reproduce this.

@melvin-bot melvin-bot bot removed the Overdue label Jan 8, 2024
@jliexpensify
Copy link
Contributor

I'm on Pixel 3a (v1.4.22-3) and I don't think I can repro this anymore.

  1. Open a chat and type :smile or :person to surface list
  2. Select the Emoji picker to reveal emoji keyboard
  3. Click outside picker
  4. I see the list

This is different to what I was seeing here

@0xmiros
Copy link
Contributor

0xmiros commented Jan 8, 2024

This is not repro anymore. I think we can close

@github-project-automation github-project-automation bot moved this from LOW to CRITICAL in [#whatsnext] #vip-vsb Jan 22, 2024
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 Monthly KSv2
Projects
No open projects
Status: CRITICAL
Development

No branches or pull requests