feat: add 👍/👎 feedback prompt under the latest Concierge response - #100454
feat: add 👍/👎 feedback prompt under the latest Concierge response#100454oqildev wants to merge 11 commits into
Conversation
Renders "Was that response useful?" with two thumbs beneath the newest Concierge-authored comment. Each thumb writes a real emoji reaction and nothing else -- the backend reads the reaction and, on a thumbs down, opens the feedback thread itself. Because the prompt is gated on that same reaction it resolves optimistically, stays resolved across reloads, and becomes eligible again if the user retracts the reaction. Eligibility is decided by getLatestConciergeFeedbackActionID, which requires the action to exist in Onyx rather than merely look like a Concierge comment. Two client-built actions are shaped identically to a finished answer -- the greeting from buildConciergeGreetingReportAction and the paced reply from buildConciergeDraftReportAction -- but never reach Onyx, and toggleEmojiReaction bails on both, so a prompt on either would render buttons that do nothing. The list also stands down entirely while a synthetic draft is on screen, since the draft's pacing status flips independently of the server write.
The prompt was suppressed whenever `isSyntheticDraftVisible` was true. That flag stays true until the paced draft's HTML equals the persisted action's HTML byte for byte, and the two are produced by different parsers -- the draft by a client-side `getParsedComment`, the persisted one by the server. When they never converge the flag never clears, so the prompt stayed hidden for the life of the mount and only appeared after navigating away and back. Observed live in the Concierge DM. Gate on the pacing status instead, which is what the neighbouring `shouldDisableContextMenuForConciergeDraft` already uses and which always settles. The window between pacing finishing and the server write landing is covered by the helper's Onyx-membership check. While here, drop the helper's fallback to an older answer. If the newest Concierge comment is not in Onyx the prompt now shows nothing at all, rather than moving back onto the previous reply and asking the user to rate a message they were not looking at.
The previous commit swapped this gate to the pacing status on the theory that `isSyntheticDraftVisible` could stick, based on a misread bug report. It does not stick: the effect above re-seeds the draft from the persisted action, so the reveal target is the persisted HTML and the two converge by construction. The swap also cost something. The pacing status clears before the reveal finishes, so the prompt could appear under an answer whose text was still animating in -- the case this gate exists to avoid. Keeps the previous commit's other change, which stands on its own: when the newest Concierge comment is absent from Onyx the helper returns nothing instead of falling back to an older answer.
Hover was being applied through the `style` callback, which only sees React Native's raw `state.hovered`. `GenericPressable` merges that with its own tracking and applies the result to `hoverStyle`, so move the fill there (and to `pressStyle`) instead. Sampling the hover circle in the design comment gives #F1EDE8 on a #FCFBF9 page, which is `theme.hoverComponentBG` on `theme.appBG`. The fill is meant to be this quiet, so the token stays. The tooltip in the same mock reads "Useful response", so rename the strings to match. Only the thumbs up label is mocked; the thumbs down wording is the symmetric counterpart and is worth confirming with design.
The design comment's hover fill samples as `hoverComponentBG`, but that mock sits on a plain white page. Over the chat surface the token is indistinguishable from its background -- not visible even at 200% zoom -- so an affordance that is meant to be seen is not. Use the default button surface instead, which is what a ghost button takes on hover. Worth confirming with design, and a one line change to put back. The glyph also sat high in the square. It was borrowing the reaction pill's text style, whose line height is tuned for a container sized by its own padding. Stretching that line box to the thumb's height centres the box but not the emoji inside it, since emoji carry a tall ascent. Dropping the line height lets the text keep its natural size and hands centring back to the container's flex alignment.
Reverts the previous commit's switch to the default button surface. The reason given for it -- that the chat surface is warmer than the page background and swallows `hoverComponentBG` -- was never measured: neither `chatItem` nor `chatContentScrollView` sets a background, so both inherit `appBG`, which is the same surface the design comment's mock sits on. That leaves the design's own token as the right one, and the codebase agrees: `SearchFiltersClearButton` is the same shape -- transparent, 28px, pill radius -- and hovers to `styles.hoveredComponentBG`. Use that style directly rather than a local copy of it, so the two stay in step. The fill is faint at 100% zoom. That is worth raising with design, not worth diverging over. Keeps the glyph centring fix from the previous commit.
The design comment asked for a hover state on these thumbs. Matching the mock's sampled fill exactly gives `hoverComponentBG`, which measures 1.13:1 against the chat surface (both verified from the running app: surface rgb(252,251,249), fill rgb(242,237,231)). On a 28px target that is below what the eye resolves, so the state Danny asked for does not read at all -- fidelity to the swatch at the cost of the feature. `buttonDefaultBG` is 1.26:1 and is already the fill on the add-reaction bubble that sits beside this prompt, so the thumbs match their neighbour and the hover is visible. No neutral fill in the light palette reaches 3:1 -- the strongest is `buttonPressedBG` at 1.76:1 -- so this is as far as the token set goes. Worth confirming with design, along with whether a 1px border would suit better: an edge reads far more readily than a fill at the same contrast.
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx bun ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
@DylanDylann 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] |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 563aa1e016
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Opening a report at a deep link or an old unread anchor renders a single pagination window. The newest Concierge reply inside that window is not the newest in the report, so the prompt attached to it and invited a rating of an answer the user had already moved past. Gate the derivation on `hasNewerActions` in both lists that compute it. Covered by a test that fails without the gate: the same seeded reply is the feedback target on a fully loaded report and is not one while newer pages are outstanding. Reported by Codex on the PR.
`renderThumb` built a nested Tooltip/Pressable/Text tree from inside the prompt's body, which hides the real render tree from the return statement and leaves the thumb's markup tangled with the prompt's reaction state. Give it explicit props instead, so the prompt reads as what it renders and the thumb owns its own styling. Flagged by the coding-standards reviewer as CLEAN-REACT-PATTERNS-4.
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
Videos look good to me. I'll run an adhoc though. For the removal flow. I see that the Concierge message has a thread already. Is this expected? I assumed that only showed up when there was a 👎 added. Also do we even want this ability? I guess if it doesn't change mess with our data in anyways and only shows the latest rating then it's fine? cc @Expensify/design as well |
|
🚧 dubielzyk-expensify has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
What's the spacing between thumbs? 12px? If so can we make it 8? If not, then just leave it. 
Nevermind. It did appear, but I wouldn't expect to see the 👍 and 👎 on the 
|
Hmm yeah I think this was my understanding as well. Agree with the rest of your comments though. |
…thumbs A thumbs down makes the backend open a thread on the rated message and post its own request for detail into it. That request is a Concierge comment like any other, so it became the newest one in the thread and the prompt attached to it -- the user was asked to rate being asked for feedback. Recognise the thread by the reaction the user left on its parent action and render nothing there. The thumbs also sat about 10px further apart than the mock. The row's gap separates the label from the thumbs, which it should, but it was also pushing the two thumbs apart on top of the padding each 28px target already carries. Group them so they sit flush, as they do in the mock: glyph to glyph goes from 22px to 14px, against the mock's 12.3px.
|
@dubielzyk-expensify @shawnborton — all three are addressed in 1 - 👍/👎 on 2 - Scope: I kept that narrow rather than suppressing the prompt in every thread, since Concierge also answers questions in threads off the DM, so tell me if you'd rather it never appear in one. 3 - Spacing: I measured it at 22px glyph to glyph, not 12, because the row's 8px gap was pushing the thumbs apart on top of the 7px each 28px target carries. 4 - Spacing fix: I grouped the thumbs so they sit flush like the mock, which takes them to 14px against the mock's 12.3px; a literal 8px would need the targets to overlap, so say the word if you'd rather I shrink them. 5 - Thread on the removal flow: that one is expected, an earlier 👎 opened it before I switched to 👍, and per the issue the thread stays as chat history while only the current rating shows. |
Can we use button-to-button measurement? What is the gap for button-to-button? If it was 8px and that feels big, let's go down to 4px. |
|
Agree with Jon and Shawn's comments. I also think for these ghost thumbs, if the button to button spacing of 4px still feels too airy, it would be fine to make them sit together with a 0px gap (since they can't both be hovered at the same time—you'd never see that they are right up against each other). |
joekaufmanexpensify
left a comment
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product. Excited for this!
|
@oqildev Please use translation from #100454 (comment) |
|
@yuwenmemon When I give a dislike, it takes quite a long time for a new feedback thread to open. Could this be a BE issue? |
|
Screen.Recording.2026-09-08.at.16.13.03.mov |
|
|
|
Screen.Recording.2026-09-08.at.16.16.49.mov |
|
@codex review |
|
@MelvinBot Please review this PR carefully |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1501bf057
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const latestConciergeComment = sortedVisibleReportActions.find( | ||
| (action) => | ||
| isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT) && action.actorAccountID === CONST.ACCOUNT_ID.CONCIERGE && !isDeletedAction(action) && !isWhisperAction(action), |
There was a problem hiding this comment.
When InitiateBankAccountUnlock fails, its optimistic Concierge ADD_COMMENT remains in REPORT_ACTIONS with errors, so this predicate still selects it even though the server never created the action. The normal context menu and reaction controls explicitly block actions with errors, but this prompt remains active and attempts AddEmojiReaction against the nonexistent server action; a thumbs-up also displays a misleading success acknowledgement. Exclude actions with errors from feedback eligibility.
Useful? React with 👍 / 👎.
| // A thumbs down makes the backend open a thread on the rated message and post its own request for | ||
| // detail into it. That request is a Concierge comment like any other, so without this the newest | ||
| // comment in the feedback thread is the request itself and the user is asked to rate being asked. | ||
| const isFeedbackThread = hasReactedWithEmoji(thumbsUp, parentReactions, currentUserAccountID) || hasReactedWithEmoji(thumbsDown, parentReactions, currentUserAccountID); |
There was a problem hiding this comment.
When a user gives a Concierge parent message a 👍 and later opens that message's thread to ask a follow-up, every subsequent Concierge reply in the thread loses its feedback prompt because the parent reaction makes isFeedbackThread true. The behavior documented immediately above only creates a backend feedback-detail thread for 👎, so a positive parent reaction is not sufficient to classify an ordinary discussion thread as a feedback thread.
Useful? React with 👍 / 👎.
|
I reviewed the diff at Three things I'd block on, one of which nobody has raised yet. 1.
|
Thanks for the reviews. The acknowledgement was driven by the press rather than by the reaction. It now requires both, which fixes two things at once: retracting the reaction from the pill row brings the prompt straight back instead of leaving the thanks up until the timer fires, and a press that `toggleEmojiReaction` silently declined no longer thanks the user for something it never wrote. The prompt also sat above the option list; it now renders after it. Eligibility now skips Concierge comments carrying errors. An optimistic comment whose write failed keeps its place in the report, and the reaction row and context menu both refuse to act on those -- the server has no such action to react to.
|
@DylanDylann @MelvinBot — four of these are fixed in 1 - 2 - Thanks shown when nothing was written: the same change covers it, since a press 3 - Prompt above the option list: I moved it to render after 4 - Failed optimistic Concierge comments: I excluded actions carrying errors from eligibility, matching what the reaction row and context menu already do. 5 - Feedback-thread gate failing open in group chats: I agree with the analysis, and I would rather move to suppressing whenever the parent action is Concierge-authored, but that removes the prompt from every Concierge thread, so I would like @dubielzyk-expensify and @shawnborton to confirm that is the outcome they want. 6 - 7 - Translations: I do not have an OpenAI key, so the locale strings in this PR are hand-written; could someone with write access run the Generate static translations workflow to replace them. 8 - Thumb spacing: it is 0px button to button now, which is the flush option @dannymcclain said was fine, so tell me if you would rather have the 4px @shawnborton suggested. |
|
I don't think I am following - can you rephrase and explain it in simpler terms? |
|
Sorry @shawnborton — that was written for the engineers. Here are the only three questions I need from design, in plain terms. 1 - Should 👍👎 ever appear inside a thread? I hid it in the feedback thread that opens after a 👎, which was the fix for Jon's screenshot, but that only holds for the person who left the 👎 — in a group chat, someone else opening that same thread would still see 👍👎 under 2 - How far apart should the thumbs sit? They are touching now, which is the option @dannymcclain said was fine, and you suggested 4px — I will use whichever you prefer. 3 - Should 👎 also say |
I think I am following now, and I think your solution makes sense to me but I think I would need a video of this in action to see for myself. Especially in the case of say a workspace room (like #admins) where it's possible for multiple people to interact with Concierge and ask Concierge questions.
Can you show us quick videos of both options? We can decide from there!
I would just follow the instructions from the issue here, cc @dubielzyk-expensify to confirm |
@DylanDylann I checked the logs, and I don't think it's the backend. Generally, I'm seeing that So it seems like the delay is somewhere after the auth commit. I would guess it's probably the Onyx update reaching the client and the client rendering it in some way. One thing to check is: when the thread is slow to appear, does it show up immediately if you refresh? |
8 participants
Explanation of Change
Renders
Was that response useful? 👍 👎beneath the newest Concierge-authored comment. Each thumb writes a real emoji reaction and nothing else — the backend reads the reaction and, on a thumbs down, opens the feedback thread itself. Because the prompt is gated on that same reaction it resolves optimistically, stays resolved across reloads, and becomes eligible again if the user retracts the reaction from the pill row.Eligibility is decided by
getLatestConciergeFeedbackActionID, which requires the action to exist in Onyx rather than merely look like a Concierge comment. Two client-built actions are shaped identically to a finished answer — the greeting frombuildConciergeGreetingReportActionand the paced reply frombuildConciergeDraftReportAction— but never reach Onyx, andtoggleEmojiReactionbails on both, so a prompt on either would render buttons that do nothing. The list also stands down while a synthetic draft is on screen, so the prompt never lands on a half-written answer.Two open points from the design discussion, neither blocking:
Useful response/Not useful response— only the thumbs-up label was mocked, so the other is the symmetric counterpart). Removing them is a one-liner.hoverComponentBG, and that is what I shipped first. Measured in the running app it is#F2EDE7on#FCFBF9— 1.13:1, which is not perceivable on a 28px target at 100% zoom (the mock reads clearly because it is a ~2.2× enlargement). I moved tobuttonDefaultBG(1.26:1), the fill the add-reaction bubble beside this prompt already uses, so the state is actually visible. No neutral fill in the light palette reaches 3:1 — the strongest isbuttonPressedBGat 1.76:1 — so if a more prominent state is wanted, a 1px border would read far better than any fill. Happy to switch back tohoverComponentBGif mock fidelity is preferred.Fixed Issues
$ #96775
PROPOSAL: #96775 (comment)
Tests
How do I submit an expense?Was that response useful? 👍 👎appears beneath it👍 1reaction pill appears, the prompt is replaced byThanks for the feedback!, and the thanks fades after a few seconds👎 1pill appears and no thread is created by the client (theN Repliesindicator arrives from the backend)Offline tests
toggleEmojiReactionwrites optimisticallyQA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.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
android-native.mp4
Android: mWeb Chrome
android-mweb.mp4
iOS: Native
ios-native.mp4
iOS: mWeb Safari
ios-mweb.MOV
MacOS: Chrome / Safari
chrome-web.mp4
safari-web.mp4