AXON-1462-Add-feedback-submission-confirmation #1380
Conversation
teg-atlassian
requested review from
Blastoplex,
amarg-at,
bhorai-atl,
bwieger-atlassian-com,
cabella-dot,
ccallcottstevens,
dchiew-atl,
jwang19-atlassian,
marcomura,
matt-lassian,
om-ukr,
sdzh-atlassian and
sspanchenko
as code owners
![atlassian[bot]](https://avatars.githubusercontent.com/in/14320?s=60&v=4){kind=small}
| </button> | ||
| <b>Thanks</b> | ||
| <br /> | ||
| <p>Your valuable feedack helps us continually improve our apps.</p> |
There was a problem hiding this comment.
🔎 Code Readability - Typo
Fix typo: "feedack" should be "feedback".
Details
📖 Explanation: There's a spelling error in the confirmation message that should be corrected for proper user communication.
| <p>Your valuable feedack helps us continually improve our apps.</p> | |
| <p>Your valuable feedback helps us continually improve our apps.</p> |
| type?: 'like' | 'dislike'; | ||
| } | ||
|
|
||
| export interface FeedbackConfirmationFormProps { |
There was a problem hiding this comment.
🔎 Code Readability - Code Duplication
Remove this duplicate interface definition - it should only exist in FeedbackConfiirmationForm.tsx.
Details
📖 Explanation: This interface is already defined in the confirmation form component and having it in both places violates the DRY principle and could lead to inconsistencies.
teg-atlassian
force-pushed
the
AXON-1462-Investigate-feedback-submission-confirmation-issue
branch
3 times, most recently
from
03cb902 to
a23cfa5
Compare
| setFeedbackType(undefined); | ||
| sendFeedback(feedbackType, feedback, canContact, includeTenMessages); | ||
| setFeedbackConfirmationVisible(true); | ||
| setTimeout(() => { |
There was a problem hiding this comment.
🔥 Code Bugs
This setTimeout creates a memory leak if the component unmounts before the timeout completes - store the timeout ID in a ref and clear it in useEffect cleanup.
| export const FeedbackConfirmationForm: React.FC<FeedbackConfirmationFormProps> = ({ onClose: onClose }) => { | ||
| return ( | ||
| <div className="form-container"> | ||
| <button |
There was a problem hiding this comment.
⚠️ Maintainability - Best Practices
Move the close button after the content and position it absolutely in the top-right corner for better UX and accessibility.
teg-atlassian
force-pushed
the
AXON-1462-Investigate-feedback-submission-confirmation-issue
branch
from
a23cfa5 to
5f92e8c
Compare
| <button | ||
| type="button" | ||
| onClick={() => onClose()} | ||
| style={{ |
There was a problem hiding this comment.
⚠️ Maintainability - Best Practices
Replace inline styles with CSS classes for consistency with other form components in the codebase.
teg-atlassian
changed the title
teg-atlassian
changed the title
teg-atlassian
force-pushed
the
AXON-1462-Investigate-feedback-submission-confirmation-issue
branch
from
5ffe744 to
5f92e8c
Compare
What Is This Change?
In rovo dev chat, if a user sends a feedback, currently, he doesn't see anything that confirms the feedback is sent in the chat view. In this pr, we have added a confirmation dialog box.
How Has This Been Tested?
Basic checks:
npm run lintnpm run testAdvanced checks:
Recommendations:
Demo
https://www.loom.com/share/51198e42230e4494b1cef7f35a17506e