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

Emoji Picker Menu Navigation #2532

Merged
merged 40 commits into from
May 3, 2021
Merged

Conversation

jasperhuangg
Copy link
Contributor

@jasperhuangg jasperhuangg commented Apr 22, 2021

Details

Added the ability to navigate through the emoji picker menu with arrow keys, and to send the highlighted emoji with [Enter]. It should function the same as the emoji picker in Slack.

  • the highlighted emoji should never be cutoff by the window
  • the first emoji is highlighted upon searching emojis
  • hovering over an emoji with your cursor will also change the currently highlighted emoji
  • if no emoji is highlighted, pressing an arrow key will highlight the first emoji
  • the cursor in the search input is not affected by arrow key presses that change the highlighted emoji

Fixed Issues

Fixes #2450

Tests

I tested this feature on Web and Desktop since those are the only two places it can actually be used, but I also tested it on Mobile Web to ensure that the touchscreen check works and that the Emoji Picker Menu doesn't crash.

  1. Opened the emoji picker. Scroll down till you can see the end of on category and the start of another.
  2. Use your mouse to hover over an emoji near this boundary.
  3. Use the up/down arrow keys to move the highlight around this border. Verified that it skips over the header.
  4. Move the highlight at the top and bottom edges of the window with the arrow keys. Verified that the menu scrolls to keep the highlighted emoji in the window.
  5. Type in search terms to filter by. Verified that the first search result is highlighted. Verified that nothing breaks when you try to go out of bounds. Verified that you can move the cursor using the appropriate arrow keys when they would move the highlight out of bounds.
  6. Pressed [Enter]. Verified that the highlighted emoji was inputted into the compose box.
  7. Repeat steps 1-6 two more times to verify that event listeners are removed and added correctly.

QA Steps

Identical to the tests above. See videos for examples.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

emoji_nav_web_demo.mp4

Desktop

emoji_nav_desktop_demo.mp4

cc @stitesExpensify

@jasperhuangg jasperhuangg self-assigned this Apr 22, 2021
@jasperhuangg jasperhuangg requested review from a team and removed request for a team April 22, 2021 08:46
@MelvinBot MelvinBot requested a review from sketchydroide April 22, 2021 08:47
@jasperhuangg jasperhuangg removed the request for review from sketchydroide April 22, 2021 08:47
@jasperhuangg jasperhuangg changed the title Emoji Picker Menu Navigation with Arrow Keys Emoji Picker Menu Navigation with Arrow Keys and Mouse Apr 22, 2021
@jasperhuangg jasperhuangg changed the title Emoji Picker Menu Navigation with Arrow Keys and Mouse Emoji Picker Menu Navigation Apr 22, 2021
@jasperhuangg jasperhuangg marked this pull request as ready for review April 23, 2021 05:20
@jasperhuangg jasperhuangg requested a review from a team as a code owner April 23, 2021 05:20
@MelvinBot MelvinBot removed the request for review from a team April 23, 2021 05:20
pecanoro
pecanoro previously approved these changes Apr 28, 2021
src/CONST.js Outdated Show resolved Hide resolved
src/pages/home/report/EmojiPickerMenu/index.js Outdated Show resolved Hide resolved
src/pages/home/report/EmojiPickerMenu/index.js Outdated Show resolved Hide resolved
src/pages/home/report/EmojiPickerMenu/index.js Outdated Show resolved Hide resolved
src/pages/home/report/EmojiPickerMenu/index.js Outdated Show resolved Hide resolved
@stitesExpensify
Copy link
Contributor

Tested and it works great! Just some code organization comments and then we should be good to go!

@jasperhuangg
Copy link
Contributor Author

Hey @stitesExpensify thanks so much for the review! I've addressed most of your concerns but have a few clarifications of my own. Let me know what you think!

@jasperhuangg
Copy link
Contributor Author

@stitesExpensify Ready for another review, thanks for your feedback!

Copy link
Contributor

@stitesExpensify stitesExpensify left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for making all of those changes

@stitesExpensify stitesExpensify merged commit 0f7864a into main May 3, 2021
@stitesExpensify stitesExpensify deleted the jasper-emojiPickerArrowKeys branch May 3, 2021 17:07
@OSBotify
Copy link
Contributor

OSBotify commented May 3, 2021

🚀 Deployed to staging in version: 1.0.35-2🚀

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

@isagoico
Copy link

isagoico commented May 3, 2021

Emoji Picker - Moving by arrow keys will eventually lose focus when reaching last row of emojis

Expected result

Focus isn't lost even after reaching the last emoji on the list.

Actual result

After reaching the last emoji on the list, the focus is not visible anymore.

Action Performed

  1. Log in to expensify.cash
  2. Navigate to a conversation
  3. Click on the emoji icon
  4. Use the arrow keys to reach the bottom of the list

Platform

Web ✔️

Build: 1.0.36-0

Notes/Images/Video

Bug5052011_20210503_160509.mp4

@isagoico
Copy link

isagoico commented May 3, 2021

@jasperhuangg Not sure if the issue above should be a deploy blocker, let me know if I should open as a separate issue.

@roryabraham
Copy link
Contributor

No need for this to be a deploy blocker, @isagoico, can you create a separate issue and tag @jasperhuangg?

@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 ✅

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.

Emoji picker - Unable to navigate the emoji picker with cursor arrows and enter does nothing
6 participants