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

Skintone Emoji Border Radius and Size #5253

Merged
merged 7 commits into from
Oct 4, 2021

Conversation

mananjadhav
Copy link
Collaborator

@mananjadhav mananjadhav commented Sep 14, 2021

@stitesExpensify Can you please review this?

Details

  • Added borderRadius to Skin tone Emoji
  • Aligned skin tone picker with rest of the emojis

Fixed Issues

$ #5247

Tests

  1. Checked Emoji Skintones on all platforms
  2. Verified the default highlighted skintone

QA Steps

Emoji Size

  1. Go to EmojiPicker
  2. Hover over any Emoji
  3. Click on Choose Skintone and hover over any skintone
  4. Verify size is same for both of them.

Emoji Retention

  1. Open EmojiPicker
  2. Click on "Change default skin tone"
  3. Choose one of the skin tones.
  4. Click on the "Change default skin tone" again
  5. Verify the selected skin tone in step 3, is highlighted by default.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Screenshot 2021-09-15 at 12 29 16 AM

Updated
web-skintone-list-alignment
web-skintone-picker-alignment

Mobile Web

mweb-skintone-picker-alignment

mweb-skintone-list-alignment

Desktop

image

Updated

desktop-skintone-picker-alignment

desktop-skintone-list-alignment

iOS

ios-skintone-picker-alignment

ios-skintone-list-alignment png

Android

android-skintone-picker-alignment

android-skintone-list-alignment

@mananjadhav mananjadhav requested a review from a team as a code owner September 14, 2021 19:11
@MelvinBot MelvinBot requested review from marcaaron and removed request for a team September 14, 2021 19:11
@shawnborton
Copy link
Contributor

Can we make it so that the buttons on this screenshot are perfectly aligned? It seems like the bottom row is shifted over to the left a bit:
image

@mananjadhav
Copy link
Collaborator Author

@shawnborton does this work?

image

image

@shawnborton
Copy link
Contributor

Looks great, but perhaps in the default state we can align the ✋ icon to the left edge of the emojis above as well?
image

@marcaaron marcaaron requested review from puneetlath and removed request for marcaaron September 14, 2021 20:14
@marcaaron
Copy link
Contributor

Taking myself off this review as it's related to #4323 also tagging in @puneetlath

@mananjadhav
Copy link
Collaborator Author

Done.

image

@kadiealexander
Copy link
Contributor

Hi @mananjadhav, I've placed this issue on hold as per this update, as we are prioritising issues related to a feature release scheduled for 9/24. As an apology for the delay, we will add a $100 bonus as a thank you for waiting.

@puneetlath
Copy link
Contributor

@mananjadhav the changes look good to me, but can you add updated screenshots from all platforms to your original comment? Then I can go ahead and approve.

@mananjadhav
Copy link
Collaborator Author

@puneetlath @stitesExpensify I would like to request a review again. I have done some cleanup of the code when fixing issues for all mobile devices.

@stitesExpensify
Copy link
Contributor

Hi @mananjadhav I can give you a review, but this is currently on hold due to internal prioritization so it cannot be merged #5247 (comment)

@stitesExpensify
Copy link
Contributor

Also, are the screenshots in the PR description current? Or are they from a previous iteration of the code?

@mananjadhav
Copy link
Collaborator Author

For Web and Desktop I’ve tagged Updated.

For the rest we weren’t solving earlier so it didn’t have. Now I’ve updated for all screenshots

@puneetlath puneetlath merged commit 625665a into Expensify:main Oct 4, 2021
@OSBotify
Copy link
Contributor

OSBotify commented Oct 4, 2021

✋ 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
Copy link
Contributor

OSBotify commented Oct 6, 2021

🚀 Deployed to staging by @puneetlath in version: 1.1.5-2 🚀

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

@OSBotify
Copy link
Contributor

OSBotify commented Oct 7, 2021

🚀 Deployed to production by @chiragsalian in version: 1.1.6-0 🚀

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

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.

7 participants