-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Android-Emoji is selected only on second tap #8221
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
Conversation
Screen.Recording.2022-03-19.at.10.24.49.mov |
Seem there is been some changes after the PR is created. Can you retest on all platforms and update new recordings. Also, I tested Android: Screen_Recording_20220323-011034_New.Expensify.mp4cc: @tgolen |
@tgolen @Santhosh-Sellavel update test on all platforms WebScreen.Recording.2022-03-24.at.01.18.32.movmWebScreen.Recording.2022-03-24.at.01.28.13.movDesktopScreen.Recording.2022-03-24.at.01.19.04.moviOSScreen.Recording.2022-03-24.at.01.25.37.movAndroidScreen.Recording.2022-03-24.at.01.41.40.mov |
PR Reviewer Checklist
|
src/components/EmojiPicker/index.js
Outdated
return; | ||
} | ||
|
||
// Dismiss the Keyboard and focus to prevent emoji press twice when selecting an emoji |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment could be better,
// Dismiss the keyboard to provide a focus for the emoji picker to avoid selection issues.
// Eg: https://github.com/Expensify/App/issues/8110
cc: @tgolen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I don't like this change. If we had a policy of a code comment referencing the GH for each issue that was fixed in the code, then the code will be littered with thousands of them. It's easy to find the associated GH for a change using "blame" to find the PR that introduced the code.
Can you please remove the GH link from the comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove link from comment! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! cc: @tgolen
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to production by @roryabraham in version: 1.1.49-1 🚀
|
Details
The issue is happen because the keyboard is still visible after the modal show, show the first press is for dismissing the keyboard then the second press is for selecting emoji.
Fixed Issues
$ #8110
Tests
PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectionsrc/languages/*
filesSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
displayName
property(i.e.
StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)PR Reviewer Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectionsrc/languages/*
filesSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
displayName
property(i.e.
StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)QA Steps
Screenshots
Web
Screen.Recording.2022-03-24.at.01.18.32.mov
mWeb
Screen.Recording.2022-03-24.at.01.28.13.mov
Desktop
Screen.Recording.2022-03-24.at.01.19.04.mov
iOS
Screen.Recording.2022-03-24.at.01.25.37.mov
Android
Screen.Recording.2022-03-24.at.01.41.40.mov