Hide emoji category picker while searching#15402
Conversation
|
@stitesExpensify @mananjadhav 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] |
|
There is no search field on mWeb/native, so I just provide a screenshot to make sure it stays the same. |
|
@shawnborton do we need more padding at the top of the search input here? Or are we good to go? 2023-02-24_08-00-23.mp4 |
|
Could you show it to me with an extra 4px on top and an extra 8px on top so we can compare? Thanks! |
|
Awesome, thank you! I think 8px is the winner for me, what do you think? |
|
That looks better. I have added the padding. |
Reviewer Checklist
Screenshots/VideosWeb2023-02-24_08-00-23.mp4Mobile Web - ChromeN/A Mobile Web - SafariN/A Desktop2023-02-24_08-05-48.mp4iOSN/A AndroidN/A |
|
Can't merge this because Jest Unit Tests job 3 isn't finishing currently. Once we get it fixed, I'll merge |
|
@stitesExpensify Quick question, if I press the down arrow key from Search it directly jumps to the Frequently Used Emojis set (or the first category of the emojis), instead of the Emojis Category. But I also see we don't have any navigation enabled on |
Reviewer Checklist
Screenshots/VideosWebweb-emoji-hide-category-bar.movDesktopdesktop-emoji-hide-category-bar.movI've approved the PR because the arrow navigation isn't a part of the scope we decided earlier and that @stitesExpensify has approved it already. But I would still wait to get it clarified before merging. I am left with iOS screenshot but having some issues with my build. |
|
We already have the arrow navigation reported here https://expensify.slack.com/archives/C049HHMV9SM/p1676484613354519?thread_ts=1676484613.354519&cid=C049HHMV9SM, but looks like it is on hold. |
|
I don't think that we need to account for navigation at this time. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Performance Comparison Report 📊Significant Changes To Duration
Show details
Meaningless Changes To DurationShow entries
Show details
|
|
@Expensify/mobile-deployers 📣 Please look into this performance regression as it's a deploy blocker. |
|
🚀 Deployed to staging by https://github.com/stitesExpensify in version: 1.2.77-0 🚀
|
|
🚀 Deployed to staging by https://github.com/stitesExpensify in version: 1.2.77-0 🚀
|
|
🚀 Deployed to production by https://github.com/yuwenmemon in version: 1.2.77-4 🚀
|







Details
Fixed Issues
$ #15132
PROPOSAL: #15132 (comment)
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
Only for large screen web/desktop
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Web
Screen.Recording.2023-02-22.at.21.11.38.mov
Mobile Web - Chrome
Mobile Web - Safari
Desktop
Screen.Recording.2023-02-22.at.20.56.22.mov
iOS
Android