Emoji Category Shortcuts Bar Tooltip #15513

akshayasalvi · 2023-02-27T17:53:49Z

Details

Fixed Issues

$ #15108
PROPOSAL: #15108 (comment)

Tests

For Desktop and Web

Open the app
Go to any chat
Click on the Emoji Icon
Hover over the category icons
Ensure that you are able to see the Tooltip
You can change the language and confirm that the tooltip shows up in the selected language.
Click on the Emoji Category Icon, and confirm it scrolls to the correct category
Attempt the steps 3-6 on a new account to see that Frequently Used category is not shown and still the steps work fine.

For mWeb and Mobile Apps

Open the app
Go to any chat
Click on the Emoji Icon
Click on the Emoji Category Icon, and confirm it scrolls to the correct category
Attempt the steps 2-4 on a new account to see that Frequently Used category is not shown and still the steps work fine.

Verify that no errors appear in the JS console

Offline tests

NA

QA Steps

For Desktop and Web

Open the app
Go to any chat
Click on the Emoji Icon
Hover over the category icons
Ensure that you are able to see the Tooltip
You can change the language and confirm that the tooltip shows up in the selected language.
Click on the Emoji Category Icon, and confirm it scrolls to the correct category
Attempt the steps 3-6 on a new account to see that Frequently Used category is not shown and still the steps work fine.

For mWeb and Mobile Apps

Open the app
Go to any chat
Click on the Emoji Icon
Click on the Emoji Category Icon, and confirm it scrolls to the correct category
Attempt the steps 2-4 on a new account to see that Frequently Used category is not shown and still the steps work fine.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Web

v1
https://user-images.githubusercontent.com/57435789/221661937-ba95e97f-1c3b-48a5-a057-21796e0356f0.mp4

v2

screen-recording-2023-03-02-at-113250-am_tGYVn3nY.mp4

v3

v4 with 4px padding

Mobile Web - Chrome

Mobile Web - Safari

Desktop

v1
https://user-images.githubusercontent.com/57435789/221662042-4ef1cfeb-6943-4df7-8764-2acfa55183b8.mp4

v2

screen-recording-2023-03-02-at-113955-am_l6VRduEh.mp4

v3

v4 with 4px padding

iOS

Android

MelvinBot · 2023-02-27T17:53:57Z

@stitesExpensify @aimane-chnaif One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

…y-bar-tooltip

akshayasalvi · 2023-02-27T20:14:06Z

@aimane-chnaif @stitesExpensify PR is ready for review. I've had some issues with iOS pod setup. I treied working it out other branch and it worked fine, but here I had to reinstall the pods. Can you confirm if you face the same issue?

aimane-chnaif

bug1.mov

When hover top and bottom part of icon, tooltip not showing though highlighted

src/components/EmojiPicker/EmojiPickerMenu/index.js

src/components/EmojiPicker/EmojiPickerMenu/index.native.js

src/libs/EmojiUtils.js

akshayasalvi · 2023-02-28T13:44:54Z

When hover top and bottom part of icon, tooltip not showing though highlighted

@aimane-chnaif That's what I wanted to highlight in this comment. Because Tooltip is attached to the View it only and hence it doesn't show up for Pressable. If we want to put on Pressable, then we'll have to wrap it around the View, pass the styles and update accordingly. Let me know if you that needs to be done.

cc - @stitesExpensify

aimane-chnaif · 2023-02-28T18:04:58Z

@akshayasalvi I think that needs to be fixed for sure

aimane-chnaif · 2023-03-01T10:33:25Z

@akshayasalvi update?

akshayasalvi · 2023-03-01T14:03:12Z

I will be online in a few hours and I will update PR later today.

…y-bar-tooltip

akshayasalvi · 2023-03-01T19:04:05Z

I requested a review again from @aimane-chnaif and it accidentally removed @stitesExpensify. PR is ready with the updates and comments addressed.

aimane-chnaif · 2023-03-02T05:04:32Z

@akshayasalvi press background should cover entire icon

bug2.mov

aimane-chnaif · 2023-03-02T05:25:18Z

@akshayasalvi you should figure out why tooltip points to center if not using shiftVertical={-8}.
It seems you're not able to fix soon.
Please update code like this:

            <Pressable
                onPress={this.props.onPress}
                onHoverIn={() => this.setState({isHighlighted: true})}
                onHoverOut={() => this.setState({isHighlighted: false})}
                style={({pressed}) => ([
                    StyleUtils.getButtonBackgroundColorStyle(getButtonState(false, pressed)),
                    styles.categoryShortcutButton,
                    this.state.isHighlighted && styles.emojiItemHighlighted,
                ])}
            >
-               <View style={styles.alignSelfCenter}>
+               <Tooltip
+                   containerStyles={[styles.flex1, styles.alignSelfStretch, styles.alignItemsCenter, styles.justifyContentCenter]}
+                   text={this.props.translate(`emojiPicker.headers.${this.props.code}`)}
+               >
                    <Icon
                        fill={themeColors.icon}
                        src={this.props.icon}
                        height={variables.iconSizeNormal}
                        width={variables.iconSizeNormal}
                    />
-               </View>
+               </Tooltip>
            </Pressable>

    categoryShortcutButton: {
        flex: 1,
        borderRadius: 8,
-       paddingTop: 2,
-       paddingBottom: 2,
        height: CONST.EMOJI_PICKER_ITEM_HEIGHT,
+       alignItems: 'center',
        justifyContent: 'center',
    },

(this code diff is against main)

akshayasalvi · 2023-03-02T06:07:48Z

@aimane-chnaif thanks for the diff, I've updated as per your requirement. But I checked the other tooltip if you closely observe there's always some spacing between the element and their tooltip, that's what I was trying to achieve.

With the current implementation

Other tooltips

aimane-chnaif · 2023-03-02T06:12:32Z

@akshayasalvi your screenshot didn't apply last style change. can you update screenshot after this apply?

    categoryShortcutButton: {
        flex: 1,
        borderRadius: 8,
-       paddingTop: 2,
-       paddingBottom: 2,
        height: CONST.EMOJI_PICKER_ITEM_HEIGHT,
+       alignItems: 'center',
        justifyContent: 'center',
    },

akshayasalvi · 2023-03-02T06:18:10Z

@aimane-chnaif The video that I uploaded again in the GH PR body consists of the style change as well. I uploaded v1 and v2 for Web and Desktop. Attaching the image also here for your reference.

aimane-chnaif · 2023-03-02T09:14:18Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android / native
- Android / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

web.mov

Mobile Web - Chrome

mchrome.mov

Mobile Web - Safari

Desktop

desktop.mov

iOS

Android

android.mov

akshayasalvi · 2023-03-02T13:37:08Z

@aimane-chnaif Based on @shawnborton's comment I've updated the spacing to 4px.

shawnborton · 2023-03-02T14:35:11Z

Thank you!

akshayasalvi · 2023-03-02T16:19:21Z

@stitesExpensify @aimane-chnaif bump on the change and tagging because @stitesExpensify was removed from the reviewer list.

akshayasalvi · 2023-03-05T17:26:05Z

@stitesExpensify were you able to check this PR? Any action items for me?

stitesExpensify · 2023-03-06T16:38:31Z

Hi, I was OOO sick for most of last week. Reviewing now

stitesExpensify

Code LGTM! Testing now

OSBotify · 2023-03-06T17:06: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.

akshayasalvi · 2023-03-07T05:21:29Z

Thanks for the approval @stitesExpensify. Hope you're feeling better now.

OSBotify · 2023-03-07T18:12:00Z

🚀 Deployed to staging by https://github.com/stitesExpensify in version: 1.2.80-0 🚀

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

OSBotify · 2023-03-09T03:33:07Z

🚀 Deployed to production by https://github.com/roryabraham in version: 1.2.80-2 🚀

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

akshayasalvi added 4 commits February 27, 2023 22:50

modify util to return code and header

14869ac

update categorybar props and tooltip handling

6409401

added the prop.code

9751f8b

displace tooltip vertically

5210895

akshayasalvi requested a review from a team as a code owner February 27, 2023 17:53

melvin-bot bot requested review from aimane-chnaif and stitesExpensify and removed request for a team February 27, 2023 17:53

akshayasalvi added 5 commits February 27, 2023 23:46

lint fixes

175c09b

refactor category bar imports

4ab3256

lint fixes

cec2481

Merge branch 'main' of github.com:akshayasalvi/App into emoji-categor…

5c77a03

…y-bar-tooltip

lint fixes

5802e4c

akshayasalvi changed the title ~~[WIP]: Emoji Category Shortcuts Bar Tooltip~~ Emoji Category Shortcuts Bar Tooltip Feb 27, 2023

aimane-chnaif reviewed Feb 28, 2023

View reviewed changes

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

aimane-chnaif reviewed Feb 28, 2023

View reviewed changes

src/components/EmojiPicker/EmojiPickerMenu/index.native.js Outdated Show resolved Hide resolved

aimane-chnaif suggested changes Feb 28, 2023

View reviewed changes

src/libs/EmojiUtils.js Outdated Show resolved Hide resolved

akshayasalvi added 4 commits March 1, 2023 20:15

Merge branch 'main' of github.com:akshayasalvi/App into emoji-categor…

25404bc

…y-bar-tooltip

wrap pressable with tooltip

e77d1bb

lint fixes

f078abf

updated comments

7f97adf

akshayasalvi requested review from aimane-chnaif and removed request for stitesExpensify March 1, 2023 19:03

akshayasalvi added 2 commits March 2, 2023 11:31

fix the tooltip alignment

d46da19

styling update

84ddd2f

add 2px spacing

aa53ea8

aimane-chnaif previously approved these changes Mar 2, 2023

View reviewed changes

update spacing to 4px

fea8a48

akshayasalvi dismissed aimane-chnaif’s stale review via fea8a48 March 2, 2023 13:35

aimane-chnaif approved these changes Mar 2, 2023

View reviewed changes

stitesExpensify self-requested a review March 2, 2023 17:19

stitesExpensify approved these changes Mar 6, 2023

View reviewed changes

stitesExpensify assigned akshayasalvi Mar 6, 2023

stitesExpensify merged commit 95cdfdb into Expensify:main Mar 6, 2023

OSBotify mentioned this pull request Mar 7, 2023

Deploy Checklist: New Expensify 2023-03-07 #15717

Closed

18 tasks

MelvinBot mentioned this pull request Mar 9, 2023

[HOLD for payment 2023-03-16] [$2000] Web - Emoji - There is no tooltip for the category picker #15108

Closed

6 tasks

bernhardoj mentioned this pull request May 18, 2023

refactor tooltip to fix double focusable component #16052

Merged

55 tasks

bernhardoj mentioned this pull request May 26, 2023

Fix emoji reaction picker and tooltip wrong position #19659

Merged

57 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Emoji Category Shortcuts Bar Tooltip #15513

Emoji Category Shortcuts Bar Tooltip #15513

akshayasalvi commented Feb 27, 2023 •

edited

Loading

MelvinBot commented Feb 27, 2023

akshayasalvi commented Feb 27, 2023

aimane-chnaif left a comment

akshayasalvi commented Feb 28, 2023

aimane-chnaif commented Feb 28, 2023

aimane-chnaif commented Mar 1, 2023

akshayasalvi commented Mar 1, 2023

akshayasalvi commented Mar 1, 2023

aimane-chnaif commented Mar 2, 2023

aimane-chnaif commented Mar 2, 2023 •

edited

Loading

akshayasalvi commented Mar 2, 2023

aimane-chnaif commented Mar 2, 2023

akshayasalvi commented Mar 2, 2023

aimane-chnaif commented Mar 2, 2023 •

edited

Loading

akshayasalvi commented Mar 2, 2023

shawnborton commented Mar 2, 2023

akshayasalvi commented Mar 2, 2023

akshayasalvi commented Mar 5, 2023

stitesExpensify commented Mar 6, 2023

stitesExpensify left a comment

OSBotify commented Mar 6, 2023

akshayasalvi commented Mar 7, 2023

OSBotify commented Mar 7, 2023

OSBotify commented Mar 9, 2023

Emoji Category Shortcuts Bar Tooltip #15513

Emoji Category Shortcuts Bar Tooltip #15513

Conversation

akshayasalvi commented Feb 27, 2023 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

MelvinBot commented Feb 27, 2023

akshayasalvi commented Feb 27, 2023

aimane-chnaif left a comment

Choose a reason for hiding this comment

akshayasalvi commented Feb 28, 2023

aimane-chnaif commented Feb 28, 2023

aimane-chnaif commented Mar 1, 2023

akshayasalvi commented Mar 1, 2023

akshayasalvi commented Mar 1, 2023

aimane-chnaif commented Mar 2, 2023

aimane-chnaif commented Mar 2, 2023 • edited Loading

akshayasalvi commented Mar 2, 2023

aimane-chnaif commented Mar 2, 2023

akshayasalvi commented Mar 2, 2023

aimane-chnaif commented Mar 2, 2023 • edited Loading

Reviewer Checklist

Screenshots/Videos

akshayasalvi commented Mar 2, 2023

shawnborton commented Mar 2, 2023

akshayasalvi commented Mar 2, 2023

akshayasalvi commented Mar 5, 2023

stitesExpensify commented Mar 6, 2023

stitesExpensify left a comment

Choose a reason for hiding this comment

OSBotify commented Mar 6, 2023

akshayasalvi commented Mar 7, 2023

OSBotify commented Mar 7, 2023

OSBotify commented Mar 9, 2023

akshayasalvi commented Feb 27, 2023 •

edited

Loading

aimane-chnaif commented Mar 2, 2023 •

edited

Loading

aimane-chnaif commented Mar 2, 2023 •

edited

Loading