Suggestion box for edit input#44720
Conversation
…uggestion-box-edit-input
…uggestion-box-edit-input
…uggestion-box-edit-input
…uggestion-box-edit-input
|
@rayane-djouah Please 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] |
|
@perunt your plan sounds good to me. |
|
@perunt - Lint is failing |
…uggestion-box-edit-input
…uggestion-box-edit-input
|
@rayane-djouah done |
|
@puneetlath it looks good to go |
|
Also, I just finished the task with hiding the suggestion box |
|
Would love to review the next PR as I have context, @puneetlath please assign it to me if possible |
puneetlath
left a comment
There was a problem hiding this comment.
Left a handful of comments, mostly about naming. Overall looks solid to me.
|
|
||
| const getSuggestions = useCallback(() => suggestionValues.suggestedEmojis, [suggestionValues]); | ||
|
|
||
| const checkIfSuggestionVisible = useCallback(() => isEmojiSuggestionsMenuVisible, [isEmojiSuggestionsMenuVisible]); |
There was a problem hiding this comment.
Our guidelines for booleans are that they should start with is or are to indicate true/false. I think the following naming makes more sense for what this is doing based on my understanding. In that it will return true/false based on whether the suggestion menu is visible.
| const checkIfSuggestionVisible = useCallback(() => isEmojiSuggestionsMenuVisible, [isEmojiSuggestionsMenuVisible]); | |
| const isSuggestionsMenuVisible = useCallback(() => isEmojiSuggestionsMenuVisible, [isEmojiSuggestionsMenuVisible]); |
There was a problem hiding this comment.
Sure, it makes sense. The only concern is that it's a callback, and it would be quite confusing to do isSuggestionsMenuVisible(), so maybe it makes sense to name it getIsSuggestionsMenuVisible?
There was a problem hiding this comment.
Ah cool. That makes sense to me.
src/pages/home/report/ReportActionCompose/SuggestionMention.tsx
Outdated
Show resolved
Hide resolved
src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx
Outdated
Show resolved
Hide resolved
|
@puneetlath can you take a look again? |
|
✋ 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 staging by https://github.com/puneetlath in version: 9.0.7-3 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/thienlnam in version: 9.0.7-4 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
|
🚀 Deployed to production by https://github.com/thienlnam in version: 9.0.7-8 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.0.8-6 🚀
|
Details
This PR updates the suggestion box (in edit mode) to appear right next to the cursor, making it work like inline autosuggestion.
Fixed Issues
$ #19643
PROPOSAL:
Tests
Tests
Testing Emoji Suggestions:
Testing Mention Suggestions:
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.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
Android: Native
2024-07-03.14.56.51.mp4
Android: mWeb Chrome
2024-07-03.15.25.43.mp4
iOS: Native
RPReplay_Final1720001461.mov
iOS: mWeb Safari
MacOS: Chrome / Safari
Untitled.mov
MacOS: Desktop
Untitled.2.mov