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

Add Emoji Picker #1991

Merged
merged 95 commits into from
Apr 15, 2021
Merged

Add Emoji Picker #1991

merged 95 commits into from
Apr 15, 2021

Conversation

stitesExpensify
Copy link
Contributor

@stitesExpensify stitesExpensify commented Mar 22, 2021

CC: @roryabraham since I'm using the modal you made

Details

This is a MVP for the emoji picker, it is just a basic picker that works on all platforms without features such as skin tone and a category jumper

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/143023

Tests/QA

  1. Hover over the emoji icon and make sure its style changes
  2. Click the icon and make sure an emoji picker pops up
  3. Scroll down and make sure the category headers are sticky
  4. Click an emoji and make sure it is added to the text field
  5. Open the picker again
  6. On web/desktop you should see a search bar, on mobile you should not
  7. On web/desktop search for something
  8. Make sure your search term is reflected in the list

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

2021-04-09_12-40-32
2021-04-09_12-24-03

2021-03-29_13-01-39.mp4

@stitesExpensify stitesExpensify self-assigned this Mar 22, 2021
src/styles/styles.js Outdated Show resolved Hide resolved
@stitesExpensify
Copy link
Contributor Author

@shawnborton I actually progressed further than expected and it's just about done (at least the basic picker stuff).
I just updated the OP with some vids, anything specific you want changed? Looking at the mockup it looks like we want the search bar as its own box rather than just sitting at the top but that's the only one really sticking out to me

@shawnborton
Copy link
Contributor

Also want to make sure @michelle-thompson as eyes on this as I think she worked on the Design Doc?

@stitesExpensify
Copy link
Contributor Author

Ah sorry I actually just found one other regression introduced by this PR too. When closing the ReportActionContextMenu, it's sliding out to the left, which is weird. You can fix by passing animationOutTiming={1} here

Fixed!

Copy link
Contributor

@roryabraham roryabraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, thanks for making those fixes. LGTM 👍

@stitesExpensify
Copy link
Contributor Author

@marcaaron Up to you for the merge!

@marcaaron marcaaron merged commit 5f7cf85 into main Apr 15, 2021
@marcaaron marcaaron deleted the stites-emojipicker2 branch April 15, 2021 23:40
@roryabraham roryabraham mentioned this pull request Apr 15, 2021
5 tasks
@isagoico
Copy link

isagoico commented Apr 17, 2021

mWeb/iOS - There are empty spaces in emoji picker.

Expected result

Should be no spaces in emoji picker. Selected picture should inset in the field.

Actual result

Blank spaces are displayed in the emoji picker.

Action performed

  1. Log in to app in iOS mWeb
  2. Go to a conversation and click on the emoji icon
  3. Scroll down the list of emojis

Platform

Issue only reproducible on iOS mWeb.

mWeb iOS ✔️

Build 1.0.24-0

Notes/images/videos

Video

@isagoico isagoico added the DeployBlockerCash This issue or pull request should block deployment label Apr 17, 2021
@isagoico
Copy link

iPad - Emoji modal stuck in top corner if keyboard expanded

Expected result

Emoji modal should open properly

Actual result

Emoji modal stuck in top corner if keyboard expanded

Action performed

  1. Install 1.0.24 build
  2. Login with gmail account
  3. Open any chat
  4. Tap on text field to expand keyboard
  5. Tap on emoji icon to open modal

Platform

Issue only reproducible on iPad

iPad / iOS ✔️

Build 1.0.24-0

Notes/images/videos

emoji.picker.mp4

image

@stitesExpensify
Copy link
Contributor Author

mWeb/iOS - There are empty spaces in emoji picker.
#1991 (comment)

This one is super weird, some of the emojis are also being repeated, my first thought is that it's a bug with react-native-web rendering a flatlist but that doesn't make a ton of sense because it works on non-mobile web

@stitesExpensify
Copy link
Contributor Author

iPad - Emoji modal stuck in top corner if keyboard expanded
#1991 (comment)

This one is interesting, it's because the keyboard is open when we set the position, but then the keyboard goes away which puts it in the wrong spot. I think it's because it's using desktop functionality since the width is so big, but I'm not sure that we want the mobile version since it's huge?

@stitesExpensify
Copy link
Contributor Author

mWeb/iOS - There are empty spaces in emoji picker.
#1991 (comment)

This one is super weird, some of the emojis are also being repeated, my first thought is that it's a bug with react-native-web rendering a flatlist but that doesn't make a ton of sense because it works on non-mobile web

Oh this is extra weird because there's a search bar!? Does the index.native file not include mobile web? I guess that makes sense, but still weird

@stitesExpensify
Copy link
Contributor Author

Okay I figured out the problem with mobile web, that emojis is a combination of different emojis in unicode and apparently mobile safari hasn't updated to work with them. I'll just remove them for now.

@OSBotify
Copy link
Contributor

🚀 [Deployed](https://github.com/Expensify/Expensify.cash
/actions/runs/765130740) 🚀 to
staging on Mon Apr 19 2021 at 22:41:56 GMT+0000 (Coordinated Universal Time)

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

@roryabraham
Copy link
Contributor

Which deploy blockers remain unresolved here? It looks like @stitesExpensify had PRs for the two comments here that should have been included in today's staging version, so I'm wondering if this should be checked off in the deploy checklist

@roryabraham
Copy link
Contributor

@isagoico, can you please retest these deploy blockers during today's QA cycle and post and update with the results (and hopefully check them off the deploy checklist if they're no longer reproducible)

@isagoico
Copy link

#1991 (comment) and #1991 (comment) were not reproducible 🎉 checking the PR off the list

@isagoico isagoico removed the DeployBlockerCash This issue or pull request should block deployment label Apr 23, 2021
@OSBotify
Copy link
Contributor

OSBotify commented May 8, 2021

🚀 Deployed to production in version: 1.0.39-5🚀

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

@MitchExpensify
Copy link
Contributor

Following these instructions: Heads up @stitesExpensify @roryabraham @deetergp This PR caused a bug here: #10356

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants