PR 3: Adding EmojiPicker to the Edit Comment #7580

mananjadhav · 2022-02-05T00:37:50Z

Details

This is the third PR to close the tagged issue. It adds the EmojiPicker to the Edit Comment. Refer the comment [HOLD for payment 2022-04-12] [$2000] Edit Comment - There's no emoji picker when editing a comment #3245 (comment)

Fixed Issues

$ #3245

Tests

Tested the EmojiPicker on Composer
Tested frequently used emojis
Tested skintone for emojis
Tested searchInput focus for EmojiPicker on Load
Tested scroll for EmojiPicker (had issues with earlier implementation)
Closing EmojiPicker doesn't shift the focus to the edit comment
EmojiPicker positioning switching between Edit comment and Composer

Verify that no errors appear in the JS console

QA Steps

Go to any chat
Select any own message
Click on Edit Comment
You'll see an Emoji icon and click on the icon
Select your emoji, and ensure that it gets entered into the comment.

Verify that no errors appear in the JS console

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

mananjadhav · 2022-02-05T10:29:46Z

@parasharrajat for ReportActionCompose, there's this code on componentDidUpdate

if (this.shouldFocusInputOnScreenFocus && this.props.isFocused
    && prevProps.modal.isVisible && !this.props.modal.isVisible) {
    this.focus();
}

It seems you're the code owner. I can't find how isFocused as passed as props and need some help in understanding the props.modal.

What is happening is, after I close EmojiPicker on any edit comment, it calls this componentDidUpdate of composer and focuses the compose input.

parasharrajat · 2022-02-05T10:38:05Z

Ok. I see. I will tell you what is the use of it and how to manage this when I will review the PR.

I am waiting on the first PR to be merged. We will go step by step.

…moji-picker-in-edit-message # Conflicts: # src/pages/home/report/ReportActionCompose.js # src/pages/home/report/ReportActionItemMessageEdit.js

…moji-picker-in-edit-message # Conflicts: # src/components/EmojiPicker/EmojiPickerButton.js

src/pages/home/report/ReportActionItemMessageEdit.js

parasharrajat · 2022-02-27T16:05:27Z

Pending: Closing EmojiPicker doesn't shift the focus to the edit comment.

For this, you can use shouldSetModalVisibility flag. If you set this to false, the focus is not set to the composer when the modal is closed.

Now, you can set the focus back to editbox same as

App/src/pages/home/report/ReportActionItemMessageEdit.js

Line 134 in 36c6f8a

() => InteractionManager.runAfterInteractions(() => this.textInput.focus()),

…moji-picker-in-edit-message

parasharrajat · 2022-03-08T14:43:59Z

Any update on PR?

…moji-picker-in-edit-message

mananjadhav · 2022-03-08T21:04:53Z

@parasharrajat @Julesssss PR updated with the feedback and bugs resolved. Can you please review it?

src/pages/home/report/ReportActionItemMessageEdit.js

parasharrajat · 2022-03-09T17:32:40Z

src/components/EmojiPicker/index.js

@@ -79,6 +79,7 @@ class EmojiPicker extends React.Component {

        this.emojiPopoverAnchor.measureInWindow((x, y, width) => this.setState({
            emojiPopoverAnchorPosition: {horizontal: x + width, vertical: y},
+            isEmojiPickerVisible: true,


This could be a problem and this is an extra render. measureEmojiPopoverAnchorPosition is tied to Dimensions Change event and thus we should not do this here. But you can use this method to get the dimensions

I had earlier not done it here if you see, I want the picker to be visible after the anchor position is calculated. With the earlier case, I faced this issue #3245 (comment)

Yup. I saw that.

But you can use this method to get the dimensions
Use this method to get dimensions only. Then where you are opening the picker and set the dimensions on state. Then create another method that will also get the dimensions from it and update the state. This new method will be used in Dimensions change.

…moji-picker-in-edit-message

parasharrajat · 2022-03-14T14:19:45Z

Any update @mananjadhav ?

mananjadhav · 2022-03-14T14:33:13Z

Working on it today. Can you confirm the state change is the only change and no other changes?

parasharrajat · 2022-03-14T14:45:01Z

Yup. Rest looks good to me.

mananjadhav · 2022-03-14T15:21:22Z

@parasharrajat PR updated

src/components/EmojiPicker/index.js

parasharrajat · 2022-03-21T20:04:13Z

@mananjadhav Could you please merge the main? I am testing it now. Sorry for the delay.

parasharrajat · 2022-03-21T20:07:38Z

@shawnborton is the margin correct for picker icon?

parasharrajat

LGTM but awaiting design review.

…moji-picker-in-edit-message

mananjadhav · 2022-03-23T07:01:34Z

@mananjadhav Could you please merge the main? I am testing it now. Sorry for the delay.

Done.

parasharrajat · 2022-03-23T07:50:30Z

@Expensify/design is the margin correct for picker icon from edges of Edit box? There is an equal gap on the left and right sides of the emoji icon.

megankelso · 2022-03-23T17:18:39Z

that looks a bit too narrow to me, I would increase it some.

michelle-thompson · 2022-03-23T17:32:51Z

Agree with Megan. Also it doesn't look centered through the middle? General rule of thumb, margins on top, bottom, and sides, should look the same.

mananjadhav · 2022-03-23T18:19:11Z

I'll fix the margin-right, in the edit mode, but vertical alignment looks fine as far as I can see. Here's the screenshot for edit and create mode. I used the same style, will 3px more on the right be fine?

Updated Margin

@parasharrajat @michelle-thompson @megankelso

…moji-picker-in-edit-message

parasharrajat

LGTM.

cc: @Julesssss

PR Reviewer Checklist

🎀 👀 🎀 C+ reviewed

Julesssss · 2022-03-30T14:40:06Z

Tests well, better emoji support than Github! 👍 😎 🥇 ☕

OSBotify · 2022-03-30T14:58:28Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2022-03-30T20:48:42Z

🚀 Deployed to staging by @Julesssss in version: 1.1.47-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2022-04-05T17:30:49Z

🚀 Deployed to production by @roryabraham in version: 1.1.49-1 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

cubuspl42 · 2023-11-08T10:31:34Z

src/pages/home/report/ReportActionItemMessageEdit.js

+                    <View style={styles.editChatItemEmojiWrapper}>
+                        <EmojiPickerButton
+                            isDisabled={shouldDisableEmojiPicker}
+                            onModalHide={() => InteractionManager.runAfterInteractions(() => this.textInput.focus())}


Coming from #30119

If, at the beginning, a composer's emoji picker is open, opening some other emoji picker results in a race:

Here, we focus the composer because the original emoji picker hides

The other emoji picker focuses its search field

mananjadhav added 3 commits February 5, 2022 05:07

feat: Added EmojiPicker in Edit Comment

105e260

feat: Added props to ReportActionItem

cfe4f80

feat: Removed onBeforeShowEmojiPicker for EmojiPicker

9e069e9

mananjadhav requested a review from a team as a code owner February 5, 2022 00:37

MelvinBot requested review from parasharrajat and puneetlath and removed request for a team February 5, 2022 00:37

mananjadhav added 3 commits February 22, 2022 23:37

Merge branch 'main' of https://github.com/mananjadhav/App into feat/e…

9106221

…moji-picker-in-edit-message # Conflicts: # src/pages/home/report/ReportActionCompose.js # src/pages/home/report/ReportActionItemMessageEdit.js

Merge branch 'main' of https://github.com/mananjadhav/App into feat/e…

401fc2b

…moji-picker-in-edit-message # Conflicts: # src/components/EmojiPicker/EmojiPickerButton.js

feat: Updated condition for edit message emojipicker

f1d8e58

parasharrajat suggested changes Feb 22, 2022

View reviewed changes

puneetlath requested review from Julesssss and removed request for puneetlath March 4, 2022 20:37

Merge branch 'main' of https://github.com/mananjadhav/App into feat/e…

15ac99b

…moji-picker-in-edit-message

mananjadhav added 4 commits March 9, 2022 00:25

Merge branch 'main' of https://github.com/mananjadhav/App into feat/e…

6f34743

…moji-picker-in-edit-message

fix: show emojipicker only aftr anchor position is measured

3fd3690

fix: add focus to edit comment on emoji selection

74ea882

fix: code cleanup

9317bcb

parasharrajat suggested changes Mar 9, 2022

View reviewed changes

src/pages/home/report/ReportActionItemMessageEdit.js Outdated Show resolved Hide resolved

src/pages/home/report/ReportActionItemMessageEdit.js Outdated Show resolved Hide resolved

fix: Removed extra space

6d5ef67

parasharrajat suggested changes Mar 9, 2022

View reviewed changes

Merge branch 'main' of https://github.com/mananjadhav/App into feat/e…

f7043d6

…moji-picker-in-edit-message

fix: split measure anchor position and update state

b5d1258

parasharrajat reviewed Mar 14, 2022

View reviewed changes

src/components/EmojiPicker/index.js Show resolved Hide resolved

fix: added this binding to measure function

202cecd

parasharrajat reviewed Mar 22, 2022

View reviewed changes

Merge branch 'main' of https://github.com/mananjadhav/App into feat/e…

eabc817

…moji-picker-in-edit-message

mananjadhav mentioned this pull request Mar 23, 2022

[HOLD for payment 2022-04-12] [$2000] Edit Comment - There's no emoji picker when editing a comment #3245

Closed

mananjadhav added 2 commits March 23, 2022 23:59

fix: added margin to emoji button in edit mode

38a602e

Merge branch 'main' of https://github.com/mananjadhav/App into feat/e…

dac1391

…moji-picker-in-edit-message

parasharrajat approved these changes Mar 28, 2022

View reviewed changes

Julesssss assigned mananjadhav Mar 30, 2022

Julesssss approved these changes Mar 30, 2022

View reviewed changes

Julesssss merged commit e7565c1 into Expensify:main Mar 30, 2022

OSBotify mentioned this pull request Mar 30, 2022

Deploy Checklist: New Expensify 2022-03-30 #8378

Closed

47 tasks

kavimuru mentioned this pull request Mar 30, 2022

[Hold for Payment] Emoji selector hides keyboard hides text input #8379

Closed

amyevans mentioned this pull request Jul 26, 2022

Web - Emoji - Emojis picker window overlaps with the compose box #10089

Closed

cubuspl42 reviewed Nov 8, 2023

View reviewed changes

cubuspl42 mentioned this pull request Nov 8, 2023

[HOLD for payment 2023-11-09] [$500] Chat - Focus on edit even when reaction emoji picker is opened #30119

Closed

6 tasks

shubham1206agra mentioned this pull request Dec 30, 2023

Fix tooltip doesn't show after closing the emoji picker #33690

Merged

50 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR 3: Adding EmojiPicker to the Edit Comment #7580

PR 3: Adding EmojiPicker to the Edit Comment #7580

mananjadhav commented Feb 5, 2022 •

edited

Loading

mananjadhav commented Feb 5, 2022

parasharrajat commented Feb 5, 2022

parasharrajat commented Feb 27, 2022

parasharrajat commented Mar 8, 2022

mananjadhav commented Mar 8, 2022

parasharrajat Mar 9, 2022 •

edited

Loading

mananjadhav Mar 10, 2022

parasharrajat Mar 10, 2022

parasharrajat commented Mar 14, 2022

mananjadhav commented Mar 14, 2022

parasharrajat commented Mar 14, 2022

mananjadhav commented Mar 14, 2022

parasharrajat commented Mar 21, 2022

parasharrajat commented Mar 21, 2022

parasharrajat left a comment •

edited

Loading

mananjadhav commented Mar 23, 2022

parasharrajat commented Mar 23, 2022 •

edited

Loading

megankelso commented Mar 23, 2022

michelle-thompson commented Mar 23, 2022

mananjadhav commented Mar 23, 2022 •

edited

Loading

parasharrajat left a comment

Julesssss commented Mar 30, 2022

OSBotify commented Mar 30, 2022

OSBotify commented Mar 30, 2022

OSBotify commented Apr 5, 2022

cubuspl42 Nov 8, 2023

PR 3: Adding EmojiPicker to the Edit Comment #7580

PR 3: Adding EmojiPicker to the Edit Comment #7580

Conversation

mananjadhav commented Feb 5, 2022 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

mananjadhav commented Feb 5, 2022

parasharrajat commented Feb 5, 2022

parasharrajat commented Feb 27, 2022

parasharrajat commented Mar 8, 2022

mananjadhav commented Mar 8, 2022

parasharrajat Mar 9, 2022 • edited Loading

Choose a reason for hiding this comment

mananjadhav Mar 10, 2022

Choose a reason for hiding this comment

parasharrajat Mar 10, 2022

Choose a reason for hiding this comment

parasharrajat commented Mar 14, 2022

mananjadhav commented Mar 14, 2022

parasharrajat commented Mar 14, 2022

mananjadhav commented Mar 14, 2022

parasharrajat commented Mar 21, 2022

parasharrajat commented Mar 21, 2022

parasharrajat left a comment • edited Loading

Choose a reason for hiding this comment

mananjadhav commented Mar 23, 2022

parasharrajat commented Mar 23, 2022 • edited Loading

megankelso commented Mar 23, 2022

michelle-thompson commented Mar 23, 2022

mananjadhav commented Mar 23, 2022 • edited Loading

parasharrajat left a comment

Choose a reason for hiding this comment

PR Reviewer Checklist

Julesssss commented Mar 30, 2022

OSBotify commented Mar 30, 2022

OSBotify commented Mar 30, 2022

OSBotify commented Apr 5, 2022

cubuspl42 Nov 8, 2023

Choose a reason for hiding this comment

mananjadhav commented Feb 5, 2022 •

edited

Loading

parasharrajat Mar 9, 2022 •

edited

Loading

parasharrajat left a comment •

edited

Loading

parasharrajat commented Mar 23, 2022 •

edited

Loading

mananjadhav commented Mar 23, 2022 •

edited

Loading