Added emoji picker in mobile app keyboard topbar #7523

harshal2030 · 2023-08-29T20:16:48Z

Summary

This PR adds emoji picker in the quick actions and merge library option with camera using bottom sheet

Ticket Link

Fixes mattermost/mattermost#24323
Fixes mattermost/mattermost#24460

Checklist

Has UI changes
Have tested against the 5 core themes to ensure consistency between them.

Device Information

This PR was tested on:
Device Name: Samsung Galaxy S20 FE
OS: Android 13

Screenshots

mm.cursor.mp4

Release Note

Adds emoji picker in the quick action and image options are merged in one bottom sheet.

mattermost-build · 2023-08-29T20:17:08Z

Hello @harshal2030,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

harshal2030 · 2023-08-29T20:20:56Z

I believe that it needs change with localization files if needed exact text from figma for image bottom sheet. I need input from your side on this, if similar text from localization can be used or needs a change.

thanks

larkox

LGTM. The only required change is the one about not needing the customEmojis. The rest I leave it to you to consider whether it make sense to address it here.

larkox · 2023-08-31T15:17:46Z

app/components/post_draft/quick_actions/input_quick_action/index.tsx

+    const handleEmojiClick = useCallback((emoji: string) => {
+        let emojiDraft: string;
+
+        const emojiData = getEmojiByName(emoji, customEmojis);
+
+        if (emojiData?.image && emojiData.category !== 'custom') {
+            const codeArray: string[] = emojiData.image.split('-');
+            const code = codeArray.reduce((acc, c) => {
+                return acc + String.fromCodePoint(parseInt(c, 16));
+            }, '');
+            emojiDraft = code;
+        } else {
+            emojiDraft = `:${emoji}: `;
+        }


A great part of this logic probably comes from components/autocomplete/emoji_suggestion/emoji_suggestion.tsx. Any chance we can extract it to a common function in the utils folder?

yeah, I feel too that it can be refactored, will do it

larkox · 2023-08-31T15:19:38Z

app/components/post_draft/quick_actions/input_quick_action/index.tsx

+    const handleEmojiClick = useCallback((emoji: string) => {
+        let emojiDraft: string;
+
+        const emojiData = getEmojiByName(emoji, customEmojis);


I feel we don't need the custom emojis at this level, since they will go all through the else part of the next condition.

So we an pass here an empty list, and for this will result in the same behavior, right?

larkox · 2023-08-31T15:21:09Z

app/components/post_draft/quick_actions/input_quick_action/index.tsx

+            emojiDraft = `:${emoji}: `;
+        }
+
+        updateValue((v) => `${v}${emojiDraft} `);


Will this always add the emoji at the end or at the cursor? Right now I feel it can stay as is if it only adds it at the end, but we will probably add another ticket to add it at the cursor.

ok, I would like to work on that, would be really helpful if you can automatically assign me when issue created

larkox

Another minor thing.

larkox · 2023-08-31T15:25:59Z

app/components/post_draft/quick_actions/image_quick_action/index.tsx

 import {changeOpacity} from '@utils/theme';

+import CameraType, {type ImageOptions} from './image_type';


The file has been renamed to image type, but you are still importing it here as camera type.

larkox · 2023-08-31T15:28:04Z

@harshal2030 Regarding the strings, you need to run npm run i18n-extract. That will add the new string to the en.json. Then there is the question about the changes on the strings.

@abhijit-singh The figma design had a different text than what we have nowadays in the app:
Capture Photo vs Take a photo
Record Video vs Record a video
Is this something we want to update?

If so, @harshal2030 you will need to update those two strings manually on en.json. The rest of the languages will be taken care of in weblate.

harshal2030 · 2023-08-31T15:31:57Z

Ok cool

abhijit-singh · 2023-09-01T11:12:48Z

@harshal2030 & @larkox We don't need to update the strings right now. What we have in the app currently works 👍

harshal2030 · 2023-09-01T20:55:09Z

@larkox I've updated code based on your comments also I found some bugs along the way such wrong cursor position on autocomplete for both @ mention and emoji. So I fixed those too in this PR. I'm attaching the screenshot before the fix to have better context.

larkox

A few minor things, but LGTM. Great work!

larkox · 2023-09-04T08:06:21Z

app/components/post_draft/quick_actions/image_quick_action/image_type.tsx

            <SlideUpPanelItem
                icon='camera-outline'
                onPress={onPhoto}
                testID='camera_type.photo'
-                text={intl.formatMessage({id: 'camera_type.photo.option', defaultMessage: 'Capture Photo'})}
+                text={intl.formatMessage({id: 'camera_type.photo.option', defaultMessage: 'Capture photo'})}


You have an unexpected change here and the next component also.

larkox · 2023-09-04T08:16:38Z

app/components/post_draft/quick_actions/input_quick_action/index.tsx

+            let emojiDraft: string;
+            const {emojiCode, emojiData} = getEmojiCodeAndData(emoji, []);
+
+            if (emojiData?.image && emojiData.category !== 'custom') {


We already have this if inside getEmojiCodeAndData so I wonder if there is any "readable and nice" way to get rid of it.

I understand we still need to know if it is a unicode emoji or just text to add or not the :, and that we don't add the : consistently in the two places this function is called, so... Happy to leave it as it is, but if you come up with a cleaner way, I am all ears 😄

ok, I'll try

larkox · 2023-09-04T08:34:29Z

app/components/post_draft/post_input/post_input.tsx

@@ -319,6 +319,7 @@ export default function PostInput({
            onChangeText={handleTextChange}
            onFocus={onFocus}
            onPaste={onPaste}
+            selection={{start: cursorPosition, end: cursorPosition}}


Not sure what this change is for.

Its for changing cursor position. Please see below

@larkox I've updated code based on your comments also I found some bugs along the way such wrong cursor position on autocomplete for both @ mention and emoji. So I fixed those too in this PR. I'm attaching the screenshot before the fix to have better context.

If we select an option in autocomplete when the cursor is between two text, the position of cursor not correct. The selection prop allow us to change cursor position

I wonder if this will mess with selections in general, since (if I am reading right) if I select some text, this will overwrite that selection, and leave it unselected. Am I right?

Yes right, I'll fix it

harshal2030 · 2023-09-05T13:38:05Z

@larkox fixed your comments, fixed selection prop too, still looking for better refactor

larkox · 2023-09-05T14:47:03Z

app/components/post_draft/post_input/post_input.tsx

@@ -319,6 +321,7 @@ export default function PostInput({
            onChangeText={handleTextChange}
            onFocus={onFocus}
            onPaste={onPaste}
+            selection={canUpdateCursorPosition ? {start: cursorPosition, end: cursorPosition} : undefined}


I don't completely get the idea here. Can you explain it?

I see that you set the canUpdateCursorPosition state based on who is updating the cursor position, but it would feel that the moment one of the systems sets this to true, we stop being able to select some part of the input.

So, the need of updating cursor is when the user uses autocomplete or pick emoji. Whenever user, does the same, canUpdateCursorPosition sets to true but then after when user selects or type something, it changes to false and we no longer control selection. This way the selection doesn't gets over written

@yasserfaraazkhan Please give some love to this part of the functionality when you get to QA this.

enahum · 2023-09-07T13:05:31Z

/update-branch

harshal2030 · 2023-09-11T11:01:27Z

@larkox I've added one more issue to this PR, please review and let me know if anything needs change

mattermost-build · 2023-09-11T12:14:47Z

Building app in separate branch.

enahum · 2023-09-12T13:41:16Z

app/utils/emoji/helpers.ts

@@ -370,3 +369,24 @@ export const searchEmojis = (fuse: Fuse<string>, searchTerm: string) => {

    return [];
 };
+
+export const getEmojiCode = (emoji: string, customEmojis: CustomEmojiModel[] = [], ios = false) => {


the last argument can be removed and use the Platform module to identify if is iOS

enahum · 2023-09-12T13:41:33Z

app/utils/emoji/helpers.ts

+
+    // We are going to set a double : on iOS to prevent the auto correct from taking over and replacing it
+    // with the wrong value, this is a hack but I could not found another way to solve it
+    const prefix = ios ? '::' : ':';


Suggested change

const prefix = ios ? '::' : ':';

const prefix = Platform.OS === 'ios' ? '::' : ':';

enahum · 2023-09-12T13:42:15Z

app/components/autocomplete/emoji_suggestion/emoji_suggestion.tsx

-        if (Platform.OS === 'ios') {
-            prefix = '::';
-        }
+        const isIOS = Platform.OS === 'ios';


not needed, see the comment on getEmojiCode utility function

enahum · 2023-09-12T13:46:54Z

app/components/post_draft/quick_actions/image_quick_action/image_type.tsx

+    const onLibrary = async () => {
+        const options: ImageOptions = {
+            mediaType: 'photo',
+            saveToPhotos: true,


this is not needed

harshal2030 · 2023-09-13T20:25:24Z

@enahum fixed your comments, let me know if anything needs change

enahum · 2023-09-14T00:02:51Z

app/components/post_draft/quick_actions/image_quick_action/image_type.tsx

+            <SlideUpPanelItem
+                icon='image-outline'
+                onPress={onLibrary}
+                testID=''


harshal2030 · 2023-09-20T08:44:17Z

Hi, there's been no update on this PR for a long time, is there anything I need to change?

enahum

Is this PR missing the check for the config to determine if the emoji picker is enabled or not? I couldn't find it anywhere

harshal2030 · 2023-09-20T12:39:50Z

@enahum can you please more context on the config? I don't fully understand

enahum · 2023-09-20T12:56:24Z

@enahum can you please more context on the config? I don't fully understand

In the system console we have a setting to determine if the emoji picker should be enabled or not https://docs.mattermost.com/configure/site-configuration-settings.html#emoji-enablepicker

If disabled, we should not show the option to open the emoji picker added in this PR

harshal2030 · 2023-09-20T12:59:26Z

ok, got it

abhijit-singh

Thanks @harshal2030! Awesome work! The basic functionality looks good but I can see a few weird issues here and there. Will try to explain them here and also attaching a screencap:

After adding an emoji from the new emoji picker for the first time once you open a particular channel, the input cursor lands before the emoji. Expectation - the cursor should be placed after the emoji.
Also, if you add more emojis after that, the placement is not consistent. They should ideally be placed one after the other. (Notice where each successive emoji gets added in the screencap attached)
After adding an emoji, if you try to type something, the first character gets deleted automatically.

Screen.cap.1.mp4

Screen.cap.2.mp4

minhthu-bic · 2023-09-26T08:49:26Z

This Emoji picker looks like a Reaction modal. I've created a Emoji keyboard looks like video below. If you want, I will create a PR.

EmojiKeyboard.mov

enahum · 2023-09-26T08:54:59Z

@abhijit-singh I actually like what @minhthu-bic has done with the keyboard area, what do you think? The only change I would make is to move the custom emojis to the end of the list.

mattermost-build · 2023-10-07T01:02:48Z

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

mm-cloud-bot added the release-note label Aug 29, 2023

mattermost-build added the Contributor label Aug 29, 2023

larkox self-requested a review August 31, 2023 15:08

larkox requested changes Aug 31, 2023

View reviewed changes

larkox requested a review from abhijit-singh August 31, 2023 15:22

larkox added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester 2: UX Review Requires review by a UX Designer labels Aug 31, 2023

larkox reviewed Aug 31, 2023

View reviewed changes

larkox approved these changes Sep 4, 2023

View reviewed changes

larkox requested a review from enahum September 4, 2023 08:35

larkox reviewed Sep 5, 2023

View reviewed changes

cwarnermm added the Docs/Needed Requires documentation label Sep 6, 2023

abhijit-singh added the Build Apps for PR Build the mobile app for iOS and Android to test label Sep 11, 2023

mattermost-build removed the Build Apps for PR Build the mobile app for iOS and Android to test label Sep 11, 2023

enahum requested changes Sep 12, 2023

View reviewed changes

enahum approved these changes Sep 14, 2023

View reviewed changes

enahum reviewed Sep 20, 2023

View reviewed changes

amyblais removed the 2: Dev Review Requires review by a core commiter label Sep 20, 2023

abhijit-singh requested changes Sep 20, 2023

View reviewed changes

mattermost-build added the Lifecycle/1:stale label Oct 7, 2023

harshal2030 closed this Nov 15, 2023

harshal2030 force-pushed the main branch from 707e59c to f0334d8 Compare November 15, 2023 07:07

amyblais removed the Docs/Needed Requires documentation label Dec 6, 2023

marianunez mentioned this pull request Dec 19, 2023

RN: Add emoji picker in mobile app keyboard topbar mattermost/mattermost#24323

Open

		import {changeOpacity} from '@utils/theme';

		import CameraType, {type ImageOptions} from './image_type';

	const prefix = ios ? '::' : ':';
	const prefix = Platform.OS === 'ios' ? '::' : ':';

Added emoji picker in mobile app keyboard topbar #7523

Added emoji picker in mobile app keyboard topbar #7523

Conversation

harshal2030 commented Aug 29, 2023 • edited Loading

Summary

Ticket Link

Checklist

Device Information

Screenshots

Release Note

mattermost-build commented Aug 29, 2023

harshal2030 commented Aug 29, 2023

larkox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harshal2030 Aug 31, 2023 • edited Loading

Choose a reason for hiding this comment

larkox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larkox commented Aug 31, 2023

harshal2030 commented Aug 31, 2023 • edited Loading

abhijit-singh commented Sep 1, 2023

harshal2030 commented Sep 1, 2023

larkox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harshal2030 Sep 4, 2023 • edited Loading

Choose a reason for hiding this comment

harshal2030 Sep 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harshal2030 commented Sep 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enahum commented Sep 7, 2023

harshal2030 commented Sep 11, 2023

mattermost-build commented Sep 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harshal2030 commented Sep 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harshal2030 commented Sep 20, 2023

enahum left a comment

Choose a reason for hiding this comment

harshal2030 commented Sep 20, 2023

enahum commented Sep 20, 2023

harshal2030 commented Sep 20, 2023

abhijit-singh left a comment

Choose a reason for hiding this comment

minhthu-bic commented Sep 26, 2023

enahum commented Sep 26, 2023

mattermost-build commented Oct 7, 2023

harshal2030 commented Aug 29, 2023 •

edited

Loading

harshal2030 Aug 31, 2023 •

edited

Loading

harshal2030 commented Aug 31, 2023 •

edited

Loading

harshal2030 Sep 4, 2023 •

edited

Loading

harshal2030 Sep 4, 2023 •

edited

Loading