[HOLD for Payment 2024-09-6][$250] clicking enter on emoji selector for status results in leaving status pane #47272

m-natarajan · 2024-08-12T22:39:07Z

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.19-2
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
Expensify/Expensify Issue URL:
Issue reported by: @dylanexpensify
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1723475125409519

Action Performed:

Head to status
Click on emoji selector
Search for any emoji
When the emoji you want is auto-focused, hit enter

Expected Result:

emoji is selected and window for emoji search disappears

Actual Result:

emoji is not selected, and the RHP for status disappears

Workaround:

unknown

Platforms:

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

Screenshots/Videos

Recording.435.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01d3caa8f59dbe1758
Upwork Job ID: 1823486563046416835
Last Price Increase: 2024-08-13
Automatic offers:

rayane-djouah | Reviewer | 103586699
Krishna2323 | Contributor | 103586701

Issue Owner

Current Issue Owner: @mallenexpensify

melvin-bot · 2024-08-12T22:39:17Z

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

Krishna2323 · 2024-08-12T22:58:40Z

Edited by proposal-police: This proposal was edited at 2024-08-12 23:02:25 UTC.

Proposal

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

clicking enter on emoji selector for status results in leaving status pane

What is the root cause of that problem?

Form is submitted because of enter press.

App/src/pages/settings/Profile/CustomStatus/StatusPage.tsx

Lines 167 to 176 in adaf5cd

    
           <FormProvider 
        
               formID={ONYXKEYS.FORMS.SETTINGS_STATUS_SET_FORM} 
        
               style={[styles.flexGrow1, styles.flex1]} 
        
               ref={formRef} 
        
               submitButtonText={translate('statusPage.save')} 
        
               submitButtonStyles={[styles.mh5, styles.flexGrow1]} 
        
               onSubmit={updateStatus} 
        
               validate={validateForm} 
        
               enabledWhenOffline 
        
           >

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

We should check if the emoji picker is active or not and if it is we will disable press on enter.

disablePressOnEnter={EmojiPickerAction.isEmojiPickerVisible()}

What alternative solutions did you explore? (Optional)

We can focus on the message input box when the modal hides.

App/src/pages/settings/Profile/CustomStatus/StatusPage.tsx

Line 189 in adaf5cd

onModalHide={() => {}}

onModalHide={() => {
    inputRef.current?.focus();
}}

What alternative solutions did you explore? (Optional 2)

We can use useKeyboardShortcut to capture enter clicks and perform emoji selection.

    useKeyboardShortcut(
        CONST.KEYBOARD_SHORTCUTS.ENTER,
        () => {
            let indexToSelect = focusedIndex;
            if (highlightFirstEmoji) {
                indexToSelect = 0;
            }

            const item = filteredEmojis[indexToSelect];
            if (!item) {
                return;
            }
            if ('types' in item || 'name' in item) {
                const emoji = typeof preferredSkinTone === 'number' && item?.types?.[preferredSkinTone] ? item?.types?.[preferredSkinTone] : item.code;
                onEmojiSelected(emoji, item);
            }
        },
        {isActive: !isEnterWhileComposition(keyBoardEvent), shouldPreventDefault: true, shouldStopPropagation: true},
    );

App/src/components/EmojiPicker/EmojiPickerMenu/index.tsx

Lines 145 to 164 in 1eef411

    
           if (!isEnterWhileComposition(keyBoardEvent) && keyBoardEvent.key === CONST.KEYBOARD_SHORTCUTS.ENTER.shortcutKey) { 
        
               let indexToSelect = focusedIndex; 
        
               if (highlightFirstEmoji) { 
        
                   indexToSelect = 0; 
        
               } 
        
               const item = filteredEmojis[indexToSelect]; 
        
               if (!item) { 
        
                   return; 
        
               } 
        
               if ('types' in item || 'name' in item) { 
        
                   const emoji = typeof preferredSkinTone === 'number' && item?.types?.[preferredSkinTone] ? item?.types?.[preferredSkinTone] : item.code; 
        
                   onEmojiSelected(emoji, item); 
        
               } 
        
               // On web, avoid this Enter default input action; otherwise, it will add a new line in the subsequently focused composer. 
        
               keyBoardEvent.preventDefault(); 
        
               // On mWeb, avoid propagating this Enter keystroke to Pressable child component; otherwise, it will trigger the onEmojiSelected callback again. 
        
               keyBoardEvent.stopPropagation(); 
        
               return; 
        
           }

daledah · 2024-08-13T04:24:18Z

Edited by proposal-police: This proposal was edited at 2024-08-13 04:48:25 UTC.

Proposal

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

Emoji is not selected, and the RHP for status disappears

What is the root cause of that problem?

In FormProvider:

App/src/pages/settings/Profile/CustomStatus/StatusPage.tsx

Line 167 in adaf5cd

<FormProvider

the disablePressOnEnter param is false by default.

When we open emoji picker then press on Enter, the onSubmit function is triggered:

App/src/pages/settings/Profile/CustomStatus/StatusPage.tsx

Line 173 in adaf5cd

onSubmit={updateStatus}

so the RHP is closed.

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

When the emoji picker is showing, we shouldn't trigger updateStatus on Enter:

App/src/pages/settings/Profile/CustomStatus/StatusPage.tsx

Line 98 in adaf5cd

const updateStatus = useCallback(

by adding an early return logic:

            if(EmojiPickerAction.emojiPickerRef.current?.isEmojiPickerVisible){
                return
            }

bellow:

App/src/pages/settings/Profile/CustomStatus/StatusPage.tsx

Line 99 in adaf5cd

    
           ({emojiCode, statusText}: FormOnyxValues<typeof ONYXKEYS.FORMS.SETTINGS_STATUS_SET_FORM>) => {

What alternative solutions did you explore? (Optional)

We can create a new state:

    const [isEmojiPickerVisible, setIsEmojiPickerVisible] = useState(false);

Then:

App/src/pages/settings/Profile/CustomStatus/StatusPage.tsx

Line 189 in adaf5cd

onModalHide={() => {}}

will be:

                            onModalHide={() => setIsEmojiPickerVisible(false)}
                            onWillShow={() => setIsEmojiPickerVisible(true)}

Then:

App/src/components/EmojiPicker/EmojiPickerButtonDropdown.tsx

Line 57 in adaf5cd

() => {},

will be:

            onWillShow,

Then:

App/src/pages/settings/Profile/CustomStatus/StatusPage.tsx

Line 167 in adaf5cd

<FormProvider

will be:

                disablePressOnEnter={isEmojiPickerVisible}

daledah · 2024-08-13T04:49:27Z

Proposal updated

melvin-bot · 2024-08-13T22:27:31Z

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

melvin-bot · 2024-08-13T22:27:37Z

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

mallenexpensify · 2024-08-13T22:29:38Z

Good catch @dylanexpensify I was able to reproduce. @rayane-djouah , plz review the proposals above.

2024-08-13_15-27-05.mp4

Krishna2323 · 2024-08-13T22:55:05Z

Proposal Updated

Minor fix

rayane-djouah · 2024-08-14T13:25:14Z

@Krishna2323 @daledah - Thank you for your proposals.

Could you clarify why, on this page for example: https://staging.new.expensify.com/search/filters/category (using SearchMultipleSelectionPicker), when you search for a category and press the Enter key, the focused category is correctly selected and the form is not submitted (even though we haven't disabled pressOnEnter for the button)? However, on the status page, if you search for an emoji and press Enter, the form is submitted (the form submit button is pressed) instead of selecting the focused emoji?

Screen.Recording.2024-08-14.at.2.14.11.PM.mov

Krishna2323 · 2024-08-14T13:37:53Z

@rayane-djouah, I believe that's because on status page when we search for emoji, the emoji is just highlighted not focused. But on categories page the option is both focused and highlighted.

rayane-djouah · 2024-08-14T13:43:29Z

@rayane-djouah, I believe that's because on status page when we search for emoji, the emoji is just highlighted not focused. But on categories page the option is both focused and highlighted.

@Krishna2323 How can we make the emoji focused to be selected correctly on enter key press instead of submitting the form?

Krishna2323 · 2024-08-14T14:11:41Z

@rayane-djouah, I think my proposal has the simplest solution possible for this because if we try to focus emoji on text input change, the focus from the input will we removed on every character entered. You can try focusing on emoji by up/down arrow keys, you will notice the focus from input is removed.

Krishna2323 · 2024-08-14T14:31:45Z

Proposal Updated

Added alternative 2

cc: @rayane-djouah

Krishna2323 · 2024-08-16T09:42:51Z

@rayane-djouah, friendly bump for checking comments above ^

rayane-djouah · 2024-08-16T12:45:25Z

@Krishna2323 - I want to make sure we understand the root cause. Could you please explain why the keyDownHandler is not being called before the form button’s pressOnEnter event handler? Is this possibly related to how we are subscribing to it here?

Krishna2323 · 2024-08-16T13:29:18Z

Could you please explain why the keyDownHandler is not being called before the form button’s pressOnEnter event handler?

@rayane-djouah, keyDownHandler is called before button’s pressOnEnter but the problem is that both event listeners are called.

What I believe is happening is that in EmojiPickerMenu, we are directly adding the listener to the document, while in the Button component, we are using useKeyboardShortcut, which utilizes KeyCommand from react-native-key-command. Due to this, the listeners are applied differently, and as a result, keyBoardEvent.preventDefault(); and keyBoardEvent.stopPropagation(); do not work as expected.

App/src/components/Button/index.tsx

Lines 149 to 160 in 578006f

    
           const config = useMemo( 
        
               () => ({ 
        
                   isActive: pressOnEnter && !shouldDisableEnterShortcut && isFocused, 
        
                   shouldBubble: allowBubble, 
        
                   priority: enterKeyEventListenerPriority, 
        
                   shouldPreventDefault: false, 
        
               }), 
        
               // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps 
        
               [shouldDisableEnterShortcut, isFocused], 
        
           ); 
        
           useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ENTER, keyboardShortcutCallback, config);

App/src/libs/KeyboardShortcut/index.ts

Line 2 in 578006f

import * as KeyCommand from 'react-native-key-command';

App/src/libs/KeyboardShortcut/index.ts

Line 92 in 578006f

    
           KeyCommand.addListener(shortcutTrigger, (keyCommandEvent, event) => bindHandlerToKeydownEvent(getDisplayName, eventHandlers, keyCommandEvent, event));

rayane-djouah · 2024-08-19T10:35:33Z

@Krishna2323's proposal looks good to me. I prefer using the 2nd alternative solution (using useKeyboardShortcut) as it fixes the root cause

🎀👀🎀 C+ reviewed

melvin-bot · 2024-08-19T10:35:44Z

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

melvin-bot · 2024-08-19T13:26:47Z

📣 @rayane-djouah 🎉 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

melvin-bot · 2024-08-19T13:26:51Z

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

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

mallenexpensify · 2024-08-31T00:10:41Z

Deployed to production earlier today
Wonder if automation isn't working here (maybe I shoulda waited til Monday before updating the labels)

rayane-djouah · 2024-08-31T08:11:56Z

Note

The production deploy automation failed: This should be on [HOLD for Payment 2024-09-6] according to #48221 prod deploy checklist, confirmed in #47761 (comment).

cc @mallenexpensify

mallenexpensify · 2024-09-02T23:25:08Z

Thanks @rayane-djouah , and for linking the deploy checklist too

@rayane-djouah , you might know the answer here in the related Slack thread.
https://expensify.slack.com/archives/C01GTK53T8Q/p1725320002440979?thread_ts=1725050431.962859&cid=C01GTK53T8Q

melvin-bot · 2024-09-06T18:12:00Z

@carlosmiceli, @mallenexpensify, @rayane-djouah, @Krishna2323 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

mallenexpensify · 2024-09-07T00:56:36Z

Contributor: @Krishna2323 paid $250 via Upwork
Contributor+: @rayane-djouah paid $250 via Upwork, can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~01d3caa8f59dbe1758

@rayane-djouah , can you also fill out the BZ checklist plz? thx

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:

[@] The PR that introduced the bug has been identified. Link to the PR:
[@] 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:
[@] 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:
[@] 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.
[@bz] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

rayane-djouah · 2024-09-08T19:54:50Z

Checklist

The PR that introduced the bug has been identified. Link to the PR: feat: implement useArrowKeyFocusManager in EmojiPickerMenu #34581
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: feat: implement useArrowKeyFocusManager in EmojiPickerMenu #34581 (comment)
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. Yes
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 Proposal

Go to Settings > Status
Click on the emoji selector
Search for any emoji
When the emoji you want is auto-focused, hit enter
Verify that the emoji is selected and the message input is focused

Do we agree 👍 or 👎

rayane-djouah · 2024-09-08T19:58:07Z

@mallenexpensify - Accepted the job and completed the checklist

mallenexpensify · 2024-09-10T00:33:38Z

Thanks @rayane-djouah , I updated the comment above to denote you've been paid.

Test Case is here

https://github.com/Expensify/Expensify/issues/427110

m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 12, 2024

melvin-bot bot assigned mallenexpensify Aug 12, 2024

mallenexpensify added the External Added to denote the issue can be worked on by a contributor label Aug 13, 2024

melvin-bot bot changed the title ~~clicking enter on emoji selector for status results in leaving status pane~~ [$250] clicking enter on emoji selector for status results in leaving status pane Aug 13, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 13, 2024

melvin-bot bot assigned rayane-djouah Aug 13, 2024

melvin-bot bot assigned carlosmiceli Aug 19, 2024

carlosmiceli assigned Krishna2323 Aug 19, 2024

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 19, 2024

Krishna2323 mentioned this issue Aug 20, 2024

fix: clicking enter on emoji selector for status results in leaving status pane. #47761

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 Weekly KSv2 labels Aug 20, 2024

mallenexpensify added Daily KSv2 and removed Reviewing Has a PR in review Weekly KSv2 labels Aug 31, 2024

melvin-bot bot added the Overdue label Aug 31, 2024

melvin-bot bot removed the Overdue label Aug 31, 2024

mallenexpensify changed the title ~~[$250] clicking enter on emoji selector for status results in leaving status pane~~ [HOLD for Payment 2024-09-6][$250] clicking enter on emoji selector for status results in leaving status pane Sep 2, 2024

melvin-bot bot added the Overdue label Sep 2, 2024

melvin-bot bot added Overdue and removed Overdue labels Sep 2, 2024

melvin-bot bot removed the Overdue label Sep 7, 2024

rayane-djouah mentioned this issue Sep 8, 2024

feat: implement useArrowKeyFocusManager in EmojiPickerMenu #34581

Merged

50 tasks

melvin-bot bot added the Overdue label Sep 9, 2024

mallenexpensify closed this as completed Sep 10, 2024

melvin-bot bot removed the Overdue label Sep 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for Payment 2024-09-6][$250] clicking enter on emoji selector for status results in leaving status pane #47272

[HOLD for Payment 2024-09-6][$250] clicking enter on emoji selector for status results in leaving status pane #47272

m-natarajan commented Aug 12, 2024 •

edited by mallenexpensify

Loading

melvin-bot bot commented Aug 12, 2024

Krishna2323 commented Aug 12, 2024 •

edited

Loading

daledah commented Aug 13, 2024 •

edited by github-actions bot

Loading

daledah commented Aug 13, 2024

melvin-bot bot commented Aug 13, 2024

melvin-bot bot commented Aug 13, 2024

mallenexpensify commented Aug 13, 2024

Krishna2323 commented Aug 13, 2024

rayane-djouah commented Aug 14, 2024

Krishna2323 commented Aug 14, 2024

rayane-djouah commented Aug 14, 2024

Krishna2323 commented Aug 14, 2024 •

edited

Loading

Krishna2323 commented Aug 14, 2024

Krishna2323 commented Aug 16, 2024

rayane-djouah commented Aug 16, 2024

Krishna2323 commented Aug 16, 2024

rayane-djouah commented Aug 19, 2024

melvin-bot bot commented Aug 19, 2024

melvin-bot bot commented Aug 19, 2024

melvin-bot bot commented Aug 19, 2024

mallenexpensify commented Aug 31, 2024

rayane-djouah commented Aug 31, 2024

mallenexpensify commented Sep 2, 2024 •

edited

Loading

melvin-bot bot commented Sep 6, 2024

mallenexpensify commented Sep 7, 2024 •

edited

Loading

rayane-djouah commented Sep 8, 2024

rayane-djouah commented Sep 8, 2024

mallenexpensify commented Sep 10, 2024

[HOLD for Payment 2024-09-6][$250] clicking enter on emoji selector for status results in leaving status pane #47272

[HOLD for Payment 2024-09-6][$250] clicking enter on emoji selector for status results in leaving status pane #47272

Comments

m-natarajan commented Aug 12, 2024 • edited by mallenexpensify Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Aug 12, 2024

Krishna2323 commented Aug 12, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

What alternative solutions did you explore? (Optional 2)

daledah commented Aug 13, 2024 • edited by github-actions bot Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

daledah commented Aug 13, 2024

Proposal updated

melvin-bot bot commented Aug 13, 2024

melvin-bot bot commented Aug 13, 2024

mallenexpensify commented Aug 13, 2024

Krishna2323 commented Aug 13, 2024

Proposal Updated

rayane-djouah commented Aug 14, 2024

Krishna2323 commented Aug 14, 2024

rayane-djouah commented Aug 14, 2024

Krishna2323 commented Aug 14, 2024 • edited Loading

Krishna2323 commented Aug 14, 2024

Proposal Updated

Krishna2323 commented Aug 16, 2024

rayane-djouah commented Aug 16, 2024

Krishna2323 commented Aug 16, 2024

rayane-djouah commented Aug 19, 2024

melvin-bot bot commented Aug 19, 2024

melvin-bot bot commented Aug 19, 2024

melvin-bot bot commented Aug 19, 2024

mallenexpensify commented Aug 31, 2024

rayane-djouah commented Aug 31, 2024

mallenexpensify commented Sep 2, 2024 • edited Loading

melvin-bot bot commented Sep 6, 2024

mallenexpensify commented Sep 7, 2024 • edited Loading

rayane-djouah commented Sep 8, 2024

Checklist

rayane-djouah commented Sep 8, 2024

mallenexpensify commented Sep 10, 2024

m-natarajan commented Aug 12, 2024 •

edited by mallenexpensify

Loading

Krishna2323 commented Aug 12, 2024 •

edited

Loading

daledah commented Aug 13, 2024 •

edited by github-actions bot

Loading

Krishna2323 commented Aug 14, 2024 •

edited

Loading

mallenexpensify commented Sep 2, 2024 •

edited

Loading

mallenexpensify commented Sep 7, 2024 •

edited

Loading