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

[Awaiting Payment 13th June] [$4000] Android - Edit message - keyboard is dismissed after selecting an emoji #29011

Closed
1 of 6 tasks
trjExpensify opened this issue Oct 6, 2023 · 68 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@trjExpensify
Copy link
Contributor

trjExpensify commented Oct 6, 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 steps:

  1. Send a message in any chat in App
  2. Long press the message and tap on Edit message
  3. Tap on the emoji picker
  4. Select an emoji and observe the keyboard is dismissed

Expected results:

The alphabetical keyboard should not be dismissed after selecting an emoji

Note: For the iOS Safari platform, it only focuses without opening the keyboard, which is another separate issue and is beyond the scope.

Note: No hacks, we need a proper solution to solve this issue. The speed of the keyboard showing back matters. There should be no delay before the keyboard is shown back and after the picker is closed. it should feel spontaneous.

There's extensive history on the original issue here. Please ask for any clarifying questions!

Actual Result:

Keyboard doesn't open up after selecting emoji.

Workaround:

Unknown

Platform:

Where is this issue occurring?

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

Version Number: v1.3.78-1

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers): any

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Screen_Recording_20220415-030417_New.Expensify.mp4

Issue reported by: @Tushu17
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1650039236885699

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~016af206b246b07b5a
  • Upwork Job ID: 1710314982636744704
  • Last Price Increase: 2023-10-20
  • Automatic offers:
    • getusha | Contributor | 0
@trjExpensify trjExpensify added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 6, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 6, 2023

Current assignee @trjExpensify is eligible for the Bug assigner, not assigning anyone new.

@melvin-bot
Copy link

melvin-bot bot commented Oct 6, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Oct 6, 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

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

melvin-bot bot commented Oct 6, 2023

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

@uzproblue
Copy link

uzproblue commented Oct 6, 2023

Proposal

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

After selecting an emoji the keyboard is not visible

What is the root cause of that problem?

/**
* Hide the emoji picker menu.
*
* @param {Boolean} isNavigating
*/
const hideEmojiPicker = (isNavigating) => {
if (isNavigating) {
onModalHide.current = () => {};
}
emojiPopoverAnchor.current = null;
setIsEmojiPickerVisible(false);
};

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

the function above should change focus on the input to show the keyboard after the emoji is selected:

@maxconnectAbhi
Copy link

maxconnectAbhi commented Oct 6, 2023

Proposal

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

Edit message - keyboard is dismissed after selecting an emoji

What is the root cause of that problem?

The first click will hide the emoji picker but can not open the Keyboard again

const selectEmoji = (emoji, emojiObject) => {
// Prevent fast click / multiple emoji selection;
// The first click will hide the emoji picker by calling the hideEmojiPicker() function
if (!isEmojiPickerVisible) {
return;
}
hideEmojiPicker(false);
if (_.isFunction(onEmojiSelected.current)) {
onEmojiSelected.current(emoji, emojiObject);
}
};

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

We should open the keyboard again just after selection the emoji.

We already have such code in other <compose/> components.

onModalHide prop is called as soon as EmojiModal close & focus the keyboard but it is already focussed hence following code will open the Keyboard.

                            onModalHide={() => {
                                   setIsFocused(true);
                                    focus(true);
                            }}
                            
                            onWillShow={() => {
                                if(!textInputRef.current.isFocused()) {
                                    return 
                                }

                                textInputRef.current.blur();
                            }}

What alternative solutions did you explore? (Optional)

NA

@ahmedGaber93
Copy link
Contributor

ahmedGaber93 commented Oct 6, 2023

Proposal

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

Android - Edit message - keyboard is dismissed after selecting an emoji

What is the root cause of that problem?

Calling focus when input is already focused, not trigger opening keyboard on android.

try snack

20231006200406138.mp4

the composer is focused while we type the emoji, so when modal hide and calling focus() while is already focused, it not trigger opening keyboard on android.

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

If Input is already focused, call blur then focus

if (textInput.isFocused()) {textInput.blur()}
textInput.focus();

we can do this for android only, and can focus any other view then focus input

here

function focusWithDelay(textInput: TextInput | null): FocusWithDelay {

As I see other proposal after my proposal just try to change the place where call blur(), I think we can discuss that in the PR

What alternative solutions did you explore? (Optional)

we can open keyboard programmatically by create native module to open keyboard, and use it like if input focus show keyboard else focus.
Like our exist one RNTextInputResetModule or edit it to include this method.

@s-alves10
Copy link
Contributor

s-alves10 commented Oct 6, 2023

Proposal

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

When the main composer is in focused state, the main composer gets focused after clicking emoji button and selecting emoji, but keyboard isn't opened

What is the root cause of that problem?

When clicking the emoji button, the composer doesn't get blurred. You can verify this by checking whether onBlur event is called or not. After emoji modal hides, we call textInput.focus() in order to focus the text input, but setting focus on the focused text input doesn't open the keyboard.
There is a known similar issue facebook/react-native#19366

This is the root cause

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

In order for calling focus() to open the keyboard, we need to blur the text input when emoji button clicked.

  1. Add the onPress props to the EmojiPickerButton and update the onPress handler
    onPress={() => {
    if (!EmojiPickerAction.emojiPickerRef.current.isEmojiPickerVisible) {
    EmojiPickerAction.showEmojiPicker(props.onModalHide, props.onEmojiSelected, emojiPopoverAnchor.current, undefined, () => {}, props.emojiPickerID);
    } else {
    EmojiPickerAction.emojiPickerRef.current.hideEmojiPicker();
    }
    }}
        onPress={() => {
            if (!EmojiPickerAction.emojiPickerRef.current.isEmojiPickerVisible) {
                EmojiPickerAction.showEmojiPicker(props.onModalHide, props.onEmojiSelected, emojiPopoverAnchor.current, undefined, () => {}, props.emojiPickerID);
            } else {
                EmojiPickerAction.emojiPickerRef.current.hideEmojiPicker();
            }
            if (props.onPress) {
                props.onPress();
            }
        }}
  1. Blur the composer when emoji button is pressed
    <EmojiPickerButton
    isDisabled={isBlockedFromConcierge || disabled}
    onModalHide={focus}
    onEmojiSelected={(...args) => composerRef.current.replaceSelectionWithText(...args)}
    emojiPickerID={report.reportID}
    />
        <EmojiPickerButton
            isDisabled={isBlockedFromConcierge || disabled}
            onModalHide={focus}
            onEmojiSelected={(...args) => composerRef.current.replaceSelectionWithText(...args)}
            emojiPickerID={report.reportID}
            onPress={() => composerRef.current.blur()}
        />

We can do this in ReportActionItemMessageEdit as well

This works as expected

Result
29011_android_native.mp4

What alternative solutions did you explore? (Optional)

@abdel-h66
Copy link
Contributor

abdel-h66 commented Oct 6, 2023

Proposal

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

Keyboard not showing up on edit text message after selecting an emoji

What is the root cause of that problem?

When the EmojiPicker panel is being hidden we call the following.

onModalHide={() => {
   setIsFocused(true);
   focus(true);
}}

This looks fine except that the text input is already focused and calling the focus again will not trigger the keyboard.

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

We can take advantage of showEmojiPicker property called onWillShow to blur the Text input when the Panel is being mounted, this way when the Emoji picker panel is being hidden the input will be already unfocused.

We then need to add new prop to the EmojiPickerButton and using it as the following

                        <EmojiPickerButton
                            isDisabled={props.shouldDisableEmojiPicker}
                            onModalHide={() => {
                                setIsFocused(true);
                                focus(true);
                            }}
                            onWillShow={() => {
                                if(!textInputRef.current.isFocused()) {
                                    return 
                                }

                                textInputRef.current.blur();
                            }}
                            onEmojiSelected={addEmojiToTextBox}
                            nativeID={emojiButtonID}
                            emojiPickerID={props.action.reportActionID}
                        />
Monosnap.screencast.2023-10-06.22-16-46.mp4

What alternative solutions did you explore? (Optional)

N/A

@ikevin127
Copy link
Contributor

Proposal

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

Keyboard doesn't open up after selecting emoji on both report composer and report edit composer.

What is the root cause of that problem?

The root cause of the problem happens only on Android and seems to be caused by the closing animation of the emoji picker popover:

// There is no way to disable animations, and they are really laggy, because there are so many
// emojis. The best alternative is to set it to 1ms so it just "pops" in and out
return (
<PopoverWithMeasuredContent
isVisible={isEmojiPickerVisible}
onClose={hideEmojiPicker}

<EmojiPickerButton
isDisabled={props.shouldDisableEmojiPicker}
onModalHide={() => {
setIsFocused(true);
focus(true);
}}

My assumption is that because this popover animation cannot be disabled and the 1ms animation time in / out was set this affects the focus called from onModalHide from EmojiPickerButton in a way which causes the keyboard to not show up even though the composer gets its focus back after the emoji popover is closed.

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

After getting some context by reading 300+ comments in the original issue and diving into the code this is what I came up with.

const selectEmoji = (emoji, emojiObject) => {
// Prevent fast click / multiple emoji selection;
// The first click will hide the emoji picker by calling the hideEmojiPicker() function
if (!isEmojiPickerVisible) {
return;
}
hideEmojiPicker(false);
if (_.isFunction(onEmojiSelected.current)) {
onEmojiSelected.current(emoji, emojiObject);
}
};

Within the EmojiPicker.js component mentioned above l added Keyboard.dismiss():

    const selectEmoji = (emoji, emojiObject) => {
        // Prevent fast click / multiple emoji selection;
        // The first click will hide the emoji picker by calling the hideEmojiPicker() function
        if (!isEmojiPickerVisible) {
            return;
        }

         // dismisses the keyboard before the popover close animation is finished (setState below)
         // without this Android keyboard won't show up on input focus until the user taps on the input
+        if (Platform.OS === 'android' && !Keyboard.isVisible()) {
+            Keyboard.dismiss();
+        }

        hideEmojiPicker(false);
        if (_.isFunction(onEmojiSelected.current)) {
            onEmojiSelected.current(emoji, emojiObject);
        }

this makes sure the keyboard is dismissed before the popover is closed. When the popover is closed and the composer focus is called from the EmojiPickerButton's proponModalHide then the focus will work as expected, showing the keyboard.

Result:

Android - Real device
real-alt.mp4
Android - Emulator
emulator-alt.mov

What alternative solutions did you explore? (Optional)

The OP mentions that we don't want any jumpy freezing transition between the popover closing and the keyboard showing up which happens due to this part of the code:

{!hideComposer && (props.shouldShowComposeInput || !props.isSmallScreenWidth) && (

because when the edit composer is focused the props.shouldShowComposeInput becomes false therefore the main composer container located within the ReportFooter aka ReportActionCompose is hidden from the UI which causes that jumpy freeze when the transition from when the emoji picker popover is closed to when the keyboard is shown.

To diminish that jumpy freeze I came up with the following additions to the ReportFooter component:

// -> above the component declaration, under imports

// enables LayoutAnimation for Android
+        if (Platform.OS === 'android') {
+            if (UIManager.setLayoutAnimationEnabledExperimental) {
+               UIManager.setLayoutAnimationEnabledExperimental(true);
+            }
+        }

// -> before component return

+        const previousShouldShowComposeInput = usePrevious(props.shouldShowComposeInput);

// android only, applies transition when the ReportActionCompose is being hidden
+        if (Platform.OS === 'android' && previousShouldShowComposeInput && !props.shouldShowComposeInput) {
+            LayoutAnimation.configureNext({
+                duration: CONST.LAYOUT_ANIMATED_TRANSITION, // <- 400ms added to CONST
+                update: {type: LayoutAnimation.Types.easeOut, property: LayoutAnimation.Properties.opacity},
+            });
// android only, resets LayoutAnimation by setting transition duration to 1ms when the ReportActionCompose is being shown
+        } else if (Platform.OS === 'android') {
+            LayoutAnimation.configureNext({
+                duration: 1,
+                update: {type: LayoutAnimation.Types.easeOut, property: LayoutAnimation.Properties.opacity},
+            });
+        }

For sure the duration is adjustable because 400ms might be too slow, this is just an example as to how LayoutAnimation can be used to smooth things out or at least give that sensation.

Alternative result:

Android - Real device
real.mp4
Android - Emulator
emulator.mov

@AmjedNazzal
Copy link
Contributor

Proposal

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

Android - Edit message - keyboard is dismissed after selecting an emoji

What is the root cause of that problem?

We have a couple of issues here causing the problem:

First issue is with these two codes working together:

onModalHide={() => {
setIsFocused(true);
focus(true);

useEffect(() => {
if (prevDraftMessage || !props.draftMessage) {
return;
}
focusTextInputAfterAnimation(textInputRef.current, 100);
}, [prevDraftMessage, props.draftMessage]);

The issue here is that on message edit we are focusing on the TextInput through ReportActionItem's useEffect and there is nothing blurring that focus and so when onModalHide tries to focus the TextInput since it's already focused this causes Android by it's default behaviour to not show the keyboard since the focus status of the TextInput hasn't changed.

The second issue is with this code:

const focus = focusWithDelay(textInputRef.current);

The problem here is that platforms handle operations differently, on Android when changing the current refs or re-rendering components, the ref doesn't get assigned correctly on the first render and so since we are instantly calling focusWithDelay with no regards to textInputRef.current, it will always recieve a null value for textInputRef.current so it will exit early in this part of it's code:

if (!textInput) {
return;
}

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

First of all we need to resolve the focus conflict, we can do that by simply blurring the TextInput when the EmojiPickerButton is pressed:

<EmojiPickerButton
    isDisabled={props.shouldDisableEmojiPicker}
    onPress={() => {
        textInputRef.current.blur();
}}

Next we need to resolve the null ref issue, we can do that by using useCallback to call focusWithDelay insuring it doesn't recieve a null and so having a more robust way of passing the ref:

const focus = useCallback(() => {
    const focusFunction = focusWithDelay(textInputRef.current)
    focusFunction(true);
})

Result

WhatsApp.Video.2023-10-09.at.3.20.56.AM.mp4

@melvin-bot melvin-bot bot added the Overdue label Oct 9, 2023
@parasharrajat
Copy link
Member

Thanks for all proposals. I will review this today.

@melvin-bot melvin-bot bot removed the Overdue label Oct 9, 2023
@chira37
Copy link

chira37 commented Oct 9, 2023

Proposal

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

Android - Edit message - keyboard is dismissed after selecting an emoji

What is the root cause of that problem?

when user open the emoji picker we are not giving focus to the EmojiPickerMenu View therefore input is not blurred

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

we can simply give the focus to the EmojiPickerMenu view using useRef to solve this issue

add ref to the root view of the EmojiPickerMenu

index.native.js

<View ref={emojiPickerMenuRef} style={styles.emojiPickerContainer} >

focus the EmojiPickerMenu when user open the modal using useEffect

useEffect(() => { emojiPickerMenuRef.current.focus(); }, []);

Result

Screen.Recording.2023-10-09.at.11.40.07.mov

@melvin-bot
Copy link

melvin-bot bot commented Oct 9, 2023

📣 @chira37! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@perunt
Copy link
Contributor

perunt commented Oct 10, 2023

@parasharrajat I'm not sure if it is reproducible anymore. This PR could fix this

@s-alves10
Copy link
Contributor

In my proposal, I mentioned the known issue of TextInput on Android #29011 (comment).

The point of the issue is that calling focus() doesn't open the keyboard if we dismiss the keyboard while the input is focused

@melvin-bot melvin-bot bot added the Overdue label Oct 11, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 12, 2023

@Julesssss, @trjExpensify, @parasharrajat Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@parasharrajat
Copy link
Member

Reviewing...

@melvin-bot melvin-bot bot removed the Overdue label Oct 13, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 13, 2023

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

@trjExpensify
Copy link
Contributor Author

What's the latest here?

@Julesssss
Copy link
Contributor

There is a chain of dependencies here that starts with the new architecture changes. So the issue we are currently holding on requires updates to the 2nd PR.

@ntdiary
Copy link
Contributor

ntdiary commented May 21, 2024

There is a chain of dependencies here that starts with the new architecture changes. So the issue we are currently holding on requires updates to the 2nd PR.

Uh, this issue has already been fixed by the phase1 PR(#35572). The phase2 PR(#42423) will only fix issue #24452. 😂

@trjExpensify
Copy link
Contributor Author

Wowza! Jules, you have an Android device to corroborate that? Exciting!

@Julesssss
Copy link
Contributor

Confirmed fixed 🎉

It's payday 🤑

@ntdiary
Copy link
Contributor

ntdiary commented May 22, 2024

I'm currently switching to newdot for payments, would appreciate it if you can temporarily hold the payment. 😄

@trjExpensify
Copy link
Contributor Author

Haha, certainly. Let me know here when you're done. :)

@ntdiary
Copy link
Contributor

ntdiary commented Jun 12, 2024

Haha, certainly. Let me know here when you're done. :)

Hi, @trjExpensify, I have switched to ND and smoothly received payments for the other two issues. 😄

@trjExpensify trjExpensify added Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review labels Jun 13, 2024
@melvin-bot melvin-bot bot added the Overdue label Jun 13, 2024
@trjExpensify trjExpensify changed the title [Hold #24452] [$4000] Android - Edit message - keyboard is dismissed after selecting an emoji [Awaiting Payment 13th June] [$4000] Android - Edit message - keyboard is dismissed after selecting an emoji Jun 13, 2024
@trjExpensify
Copy link
Contributor Author

Okay great! Payment summary as follows:

@ntdiary feel free to request! @getusha I've sent you an offer.

@melvin-bot melvin-bot bot removed the Overdue label Jun 13, 2024
@JmillsExpensify
Copy link

$4,000 approved for @ntdiary

@melvin-bot melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Jun 19, 2024
@trjExpensify
Copy link
Contributor Author

@getusha still waiting for you to accept the offer.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jun 24, 2024
@Julesssss
Copy link
Contributor

Bumped in Slack

@melvin-bot melvin-bot bot removed the Overdue label Jun 27, 2024
@getusha
Copy link
Contributor

getusha commented Jun 27, 2024

@trjExpensify accepted the offer

@Julesssss
Copy link
Contributor

@trjExpensify accepted the offer

Great, I think this can be closed.

@trjExpensify
Copy link
Contributor Author

I need to pay it.

@trjExpensify
Copy link
Contributor Author

Paid!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests