feat(ChatFeedback): add a four-point rating flow for conversational surfaces - #3885
feat(ChatFeedback): add a four-point rating flow for conversational surfaces#3885annamalailakshmann wants to merge 7 commits into
Conversation
🦋 Changeset detectedLatest commit: 80f784b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
🤖 Slash AI Review has been triggered. View execution logs |
|
(Review Cancelled - Superseded by a new run) |
9636485 to
2d502ea
Compare
ecd652a to
fe456d3
Compare
|
(Review Cancelled - Superseded by a new run) |
🛡️ Coverage ReportSummaryFull Coverage Details |
Component Status Update Needed🔔 Changes detected in
This helps keep our documentation up to date! 📚 |
kamaleshs-bridge4
left a comment
kamaleshs-bridge4
left a comment
There was a problem hiding this comment.
✨ Agentic PR Review ✨
UI Review
✅ 3 passed · ❌ 5 failed
| Check | Problem | Screenshot |
|---|---|---|
| ChatFeedback Default story | The Default story omits the required moodIcons prop, causing a runtime crash in ChatFeedbackMoodScale.web.tsx:152 (Cannot read properties of undefined (reading 'very-dissatisfied')). The story renders a Storybook error page instead of the component. |  |
| ChatFeedback Without auto dismiss story | The 'Without auto dismiss' story omits the required moodIcons prop, causing the same runtime crash as the Default story. |  |
| ChatFeedback Custom mood config story | The 'Custom mood config' story provides a moodConfig override but omits the required moodIcons prop, causing the same runtime crash. |  |
| ChatFeedback KitchenSink story | The KitchenSink story composes the three broken stories from ChatFeedback.stories.tsx (none of which pass moodIcons), causing a cascading React error: 'Rendered more hooks than during the previous render.' The entire KitchenSink page fails to render. |  |
| ChatFeedback Docs page | The Docs page's embedded Example canvas crashes because it renders the Default story, which omits the required moodIcons prop. The error message 'Cannot read properties of undefined (reading very-dissatisfied)' is displayed in place of the live component preview. |  |
Passing checks (3)
Usage
import { ChatFeedback } from '@razorpay/blade/components';
<ChatFeedback
moodIcons={{
'very-dissatisfied': <span>😢</span>,
'dissatisfied': <span>😕</span>,
'satisfied': <span>🙂</span>,
'very-satisfied': <span>😍</span>,
}}
onSubmit={(payload) => console.log(payload)}
onDismiss={() => setShow(false)}
/>| {...getStyledProps(rest)} | ||
| > | ||
| {/* Keyed on step so each arrival replays the entrance. */} | ||
| <BaseMotionBox key={step} motionVariants={stepInVariants}> |
There was a problem hiding this comment.
🟠 [MAJOR] · code-quality-critique · confidence: 9/10
Problem: BaseMotionBox is keyed on step to replay the entrance animation, but it is not wrapped in . Framer-motion requires AnimatePresence to defer unmounting until the exit animation completes. Without it, the old step is instantly removed from the DOM when the key changes, so the exit variant defined in stepInVariants never plays. The exit variant is dead code. ChatInput.web.tsx correctly wraps its BaseMotionBox in .
Suggestion: Wrap the keyed BaseMotionBox in .
| flexDirection="row" | ||
| alignItems="center" | ||
| flexShrink={0} | ||
| role="radiogroup" |
There was a problem hiding this comment.
🟠 [MAJOR] · code-quality-critique · confidence: 8/10
Problem: The mood scale uses role="radiogroup" with but does not implement the WAI-ARIA keyboard interaction pattern (arrow keys, roving tabindex). Every button is an independent tab stop.
Suggestion: Add roving tabindex and arrow key handler, or use native .
| </BaseBox> | ||
| ), | ||
|
|
||
| comment: () => ( |
There was a problem hiding this comment.
🟠 [MAJOR] · code-quality-critique · confidence: 8/10
Problem: The comment step has no back, cancel, or close affordance. A user who clicks 'Add more feedback' and changes their mind is trapped.
Suggestion: Add a cancel/close affordance and Escape-key handler.
| @@ -0,0 +1,8 @@ | |||
| export type { | |||
| ChatFeedbackProps, | |||
There was a problem hiding this comment.
🟠 [MAJOR] · api-decision-critique · confidence: 8/10
Problem: ChatFeedbackMoodIcons is exported from types.ts but not re-exported from index.ts. Consumers cannot type their icon set with a named type, inconsistent with exporting ChatFeedbackMood and ChatFeedbackMoodConfig.
Suggestion: Add ChatFeedbackMoodIcons to the type export list in index.ts.
| }; | ||
|
|
||
| export const Default = ChatFeedbackTemplate.bind({}); | ||
| Default.args = { question: "How's Ray doing so far?" }; |
There was a problem hiding this comment.
🟠 [MAJOR] · api-decision-critique · confidence: 9/10
Problem: All three story definitions omit the required moodIcons prop from their .args, causing a runtime crash. The Sandbox usage example also omits moodIcons.
Suggestion: Add moodIcons to each story's args and update the Sandbox example.
| size="small" | ||
| isDisabled={isDisabled || !comment.trim()} | ||
| > | ||
| Send |
There was a problem hiding this comment.
🔵 [MINOR] · api-decision-critique · confidence: 7/10
Problem: Comment submit button label 'Send' is hardcoded, no commentSubmitLabel prop for i18n.
Suggestion: Add a commentSubmitLabel prop with @default 'Send'.
| * When false, `onDismiss` is never fired automatically and you control removal. | ||
| * @default true | ||
| */ | ||
| autoDismiss?: boolean; |
There was a problem hiding this comment.
🔵 [MINOR] · api-decision-critique · confidence: 6/10
Problem: autoDismiss breaks the is-prefix convention used by isFullWidth and isDisabled on the same interface.
Suggestion: Consider renaming to isAutoDismiss for consistency.
| * Fires once when tags are submitted, and again with `comment` populated if the user | ||
| * goes on to add a free-text follow-up. | ||
| */ | ||
| onSubmit?: (payload: ChatFeedbackSubmitPayload) => void; |
There was a problem hiding this comment.
🙏🏻 [NEEDS CLARIFICATION] · api-decision-critique · confidence: 4/10
onSubmit fires twice when user adds a comment but payload has no discriminator. Is checking payload.comment !== undefined the intended signal?
| alignItems="center" | ||
| flexShrink={0} | ||
| role="radiogroup" | ||
| aria-label="Rate this experience" |
There was a problem hiding this comment.
🔵 [MINOR] · api-decision-critique · confidence: 6/10
Problem: Accessibility labels are hardcoded (Rate this experience, Back to rating, Feedback tags, Submit feedback). No props for i18n.
Suggestion: Consider adding accessibilityLabels prop or document they are intentionally fixed.
| BottomNav: 'bottomnav', | ||
| BottomNavItem: 'bottomnav-item', | ||
| Carousel: 'carousel', | ||
| ChatComposer: 'chat-composer', |
There was a problem hiding this comment.
🔵 [MINOR] · api-decision-critique · confidence: 7/10
Problem: ChatComposer is added to MetaConstants but no ChatComposer component exists in this PR. Appears to be unrelated.
Suggestion: Remove ChatComposer or add a comment.
kamaleshs-bridge4
added
the
Human Help Needed 🧑🏻💻
![rzp-slash[bot]](https://avatars.githubusercontent.com/u/7713209?s=60&v=4){kind=small}
There was a problem hiding this comment.
✨ Agentic PR Review ✨
UI Review
❌ 1 failed
| Check | Problem |
|---|---|
| ChatFeedback component visual review | Browser could not launch: Chrome exited with 'posix_fallocate64: symbol not found' — a glibc compatibility issue in the execution environment. Visual verification could not be performed. |
Usage
import { ChatFeedback } from '@razorpay/blade/components';
import type { ChatFeedbackMoodIcons } from '@razorpay/blade/components';
const moodIcons: ChatFeedbackMoodIcons = {
'very-dissatisfied': '😢',
dissatisfied: '😕',
satisfied: '🙂',
'very-satisfied': '😍',
};
<ChatFeedback
question="How's it going?"
moodIcons={moodIcons}
onMoodSelect={({ mood }) => console.log(mood)}
onSubmit={(payload) => console.log(payload)}
onDismiss={() => setShow(false)}
/>| }; | ||
|
|
||
| export const Default = ChatFeedbackTemplate.bind({}); | ||
| Default.args = { question: "How's Ray doing so far?" }; |
There was a problem hiding this comment.
🟠 [MAJOR] · code-quality-critique · confidence: 9/10
Problem: The Default, WithoutAutoDismiss, and CustomMoodConfig stories set args without the required moodIcons prop. The template spreads ...args into , so moodIcons is undefined at runtime. ChatFeedbackMoodScale accesses moodIcons[mood], which throws a TypeError. All three docs stories — and the KitchenSink that composes them — will crash on load.
Suggestion: Add a moodIcons object to each story's args (or to a shared default at the meta level). For example: const defaultMoodIcons = { 'very-dissatisfied': '😢', dissatisfied: '😕', satisfied: '🙂', 'very-satisfied': '😍' }; and set Default.args = { question: "...", moodIcons: defaultMoodIcons };.
| size="small" | ||
| isDisabled={isDisabled || !comment.trim()} | ||
| > | ||
| Send |
There was a problem hiding this comment.
🟠 [MAJOR] · api-decision-critique · confidence: 9/10
Problem: The 'Send' button label on the comment step is hardcoded as a bare string. The component exposes thanksLabel, addCommentLabel, and commentPlaceholder as customizable text props but does not expose a prop for the comment submit button text. This is inconsistent within the component itself and prevents localization of the submit button label, which a design system component should support.
Suggestion: Add a submitCommentLabel prop (e.g., default: 'Send') and use it for the button text, consistent with how thanksLabel, addCommentLabel, and commentPlaceholder are handled.
| clearTimers(); | ||
| setStep('thanks'); | ||
| if (!autoDismiss) return; | ||
| schedule(() => onDismiss?.(), theme.motion.delay.xgentle); |
There was a problem hiding this comment.
🟠 [MAJOR] · code-quality-critique · confidence: 7/10
Problem: submitComment schedules auto-dismiss with theme.motion.delay.xgentle (960ms), while goToThanks (called from submitTags) uses theme.motion.delay.long (2000ms). After the user invests effort writing a comment, the thanks confirmation flashes for under a second — including the 200ms entrance animation, the message is visible for only ~760ms before onDismiss fires. This is inconsistent with the 2s the user gets after submitting tags, and appears to be an oversight: submitComment inlines the goToThanks logic instead of calling it, so the two delays drifted apart.
Suggestion: Use the same delay as goToThanks (theme.motion.delay.long), or extract a shared dismissAfterThanks helper so both code paths cannot diverge. If a shorter delay is intentional, add a comment explaining why.
| alignItems="center" | ||
| flexShrink={0} | ||
| role="radiogroup" | ||
| aria-label="Rate this experience" |
There was a problem hiding this comment.
🟠 [MAJOR] · api-decision-critique · confidence: 7/10
Problem: The radiogroup has a hardcoded aria-label='Rate this experience' that cannot be customized and does not match the question prop rendered as visible text beside it. Screen readers announce 'Rate this experience' while sighted users see a different question (e.g., 'How are we doing so far?'), creating an accessibility mismatch. No other Blade component sets aria-label directly on BaseBox — SegmentedControl uses makeAccessible with accessibilityLabel.
Suggestion: Use the question prop as the radiogroup's accessible name — either via aria-labelledby pointing to the question Text element (preferred, since the text is visible), or via an accessibilityLabel prop on ChatFeedback forwarded to the MoodScale, mirroring the SegmentedControl pattern.
| <Box display="flex" flexDirection="column" gap="spacing.4"> | ||
| {allStories.map((Story) => { | ||
| return ( | ||
| <> |
There was a problem hiding this comment.
🔵 [MINOR] · code-quality-critique · confidence: 9/10
Problem: A fragment shorthand (<>...</>) is returned inside a .map() callback without a key prop, which triggers React's 'Each child in a list should have a unique key prop' warning for every story rendered in the KitchenSink.
Suggestion: Use <React.Fragment key={Story.storyName}> instead of the shorthand.
| @@ -0,0 +1,8 @@ | |||
| export type { | |||
There was a problem hiding this comment.
🔵 [MINOR] · api-decision-critique · confidence: 8/10
Problem: ChatFeedbackMoodIcons is exported from types.ts but is missing from the barrel re-export in index.ts. Since moodIcons is a required prop typed as ChatFeedbackMoodIcons, consumers need this type to annotate their icon sets but cannot import it from @razorpay/blade/components.
Suggestion: Add ChatFeedbackMoodIcons to the type export list in index.ts: export type { ..., ChatFeedbackMoodIcons, ... } from './types';
| flexDirection="row" | ||
| alignItems="center" | ||
| flexShrink={0} | ||
| role="radiogroup" |
There was a problem hiding this comment.
🔵 [MINOR] · code-quality-critique · confidence: 6/10
Problem: The container has role="radiogroup" but the four radio buttons are native elements with no roving tabindex and no arrow-key handler. Per the WAI-ARIA radiogroup pattern, Tab should enter the group once (landing on the checked or first radio) and arrow keys should move focus between options. Instead, each button is an individual Tab stop and arrow keys do nothing. axe-core does not flag this, but keyboard users familiar with the radiogroup pattern may find the behaviour unexpected.
Suggestion: Implement a roving tabindex: set tabIndex={0} on the selected (or first) radio and tabIndex={-1} on the rest, then add an onKeyDown handler on the radiogroup container that moves focus with Left/Right arrows, wrapping at the ends. Alternatively, add a comment documenting that this is a deliberate deviation.
|
(Review Cancelled - Superseded by a new run) |
…and an API contract
|
(Review Cancelled - Superseded by a new run) |
|
(Review Cancelled - Superseded by a new run) |
kamaleshs-bridge4
left a comment
kamaleshs-bridge4
left a comment
There was a problem hiding this comment.
✨ Agentic PR Review ✨
UI Review
✅ 7 passed
Passing checks (7)
Usage
import { ChatFeedback } from '@razorpay/blade/components';
const moodIcons = {
'very-dissatisfied': <span>😢</span>,
dissatisfied: <span>😕</span>,
satisfied: <span>🙂</span>,
'very-satisfied': <span>😍</span>,
};
<ChatFeedback
question="How's Ray doing so far?"
moodIcons={moodIcons}
onSubmit={({ mood, tags }) => record(mood, tags)}
onDismiss={() => setShowFeedback(false)}
/>| /** Tags the user selected. Empty when they submitted without picking any. */ | ||
| tags: string[]; | ||
| /** Free-text follow-up, present only when the user chose to add more. */ | ||
| comment?: string; |
There was a problem hiding this comment.
🟠 [MAJOR] · api-decision-critique · confidence: 8/10
Problem: ChatFeedbackSubmitPayload.comment is dead API surface. The field is declared (and exported in index.ts), the onSubmit JSDoc (types.ts:92) says it 'fires once when tags are submitted, and again with comment populated if the user goes on to add a free-text follow-up', and both the changeset and decisions.md state ChatFeedbackProps.comment folds free-text into the payload — but ChatFeedbackProps has no comment prop and useChatFeedback.submitTags (useChatFeedback.ts:104) calls onSubmit?.({ mood, tags }) with no comment, so the field is always undefined. The exported type and docs promise a value the implementation never delivers.
Suggestion: Either wire a comment?: string prop on ChatFeedbackProps through to the payload (so a host collecting free text can fold it in), or remove comment from ChatFeedbackSubmitPayload and correct the onSubmit JSDoc, changeset and decisions.md to match. Don't ship a public type field that can never be populated.
…al, ship default artwork
|
(Review Cancelled - Superseded by a new run) |
kamaleshs-bridge4
left a comment
kamaleshs-bridge4
left a comment
There was a problem hiding this comment.
✨ Agentic PR Review ✨
UI Review
✅ 7 passed
Passing checks (7)
Usage
import { ChatFeedback } from '@razorpay/blade/components';
<ChatFeedback question="How's Ray doing?" onSubmit={(payload) => console.log(payload)} onDismiss={() => setShow(false)} />
|
(Review Cancelled - Superseded by a new run) |
|
🤖 Slash AI Review has been triggered. View execution logs |
There was a problem hiding this comment.
✨ Agentic PR Review ✨
Status: Approved ✅
UI Review
✅ 5 passed
Passing checks (5)
| Check | Screenshot |
|---|---|
| ✅ ChatFeedback Default |  |
| ✅ ChatFeedback Without auto dismiss |  |
| ✅ ChatFeedback Custom mood config |  |
| ✅ ChatFeedback KitchenSink |  |
| ✅ ChatFeedback Interaction tests — hit targets |  |
Usage
import { ChatFeedback } from '@razorpay/blade/components';
<ChatFeedback
question="How's Ray doing so far?"
feedbackIcons={feedbackIcons}
onSubmit={(payload) => console.log('feedback', payload)}
onDismiss={() => setShow(false)}
/>
rzp-slash-public
Bot
added
the
rcore:eligible-for-auto-approval
3 participants
Description
A rating flow for chat surfaces: ask a question, take a mood on a four-point scale, follow up with tags, confirm, leave. Blade owns the machinery — radio-group semantics, per-point tooltips, hit targets, selected state. The artwork is the consumer's:
moodIconsis required, because Blade ships no set for this scale yet.📄 Full state review and API contract — states, transitions, invariants, alternatives considered, and open questions.
Changes
ChatFeedback(web) + a native stub that throws until implementedChatFeedbackCheck— the closing mark, a filled positive disc with a check, composed fromCheckIconand tokensonTagsChange/controlsRef/isSubmitHidden— the seam a host uses to collect free text in an input of its ownTwo decisions worth a reviewer's eye
The selected state lives on the button, not the glyph. It used to be three cues, all on the icon — recolour, lift, and swapping in a filled twin. With consumer artwork that may be untintable and has no twin, two of the three silently do nothing, leaving a 12% scale as the only sign a rating registered.
32px targets around 20px glyphs. Butted together, 20 + 12 gives the 32px pitch. An earlier 24px glyph left 4px of padding, so a tap a couple of pixels wide of a face recorded the adjacent rating — a wrong answer, not a near miss.
Breaking within this unreleased component
The free-text
commentstep and its Add more feedback link are removed, withaddCommentLabelandcommentPlaceholder. Free text is now the host's to collect;commentfolds it into the submit payload.ChatFeedbackSteploses'comment'.Consequence: a surface without a composer can no longer collect free text at all. Easy to reinstate if needed.
Additional Information
Third of a stack of four; based on #3884 for
motion.easing.settle.ChatComposerand a candidate icon set exist locally and are deliberately not in this PR.controlsRefis assigned during render so the handle cannot go stale — at odds with React's ref guidance and worth discussing for concurrent rendering. Noted as an open question in the contract.Component Checklist
🤖 Generated with Claude Code