MGMT-21223 change buttons behaviour#3059
MGMT-21223 change buttons behaviour#3059openshift-merge-bot[bot] merged 2 commits intoopenshift-assisted:masterfrom
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
WalkthroughThis change updates the chatbot component to preserve messages and conversation ID when the chatbot window is closed. Additionally, it introduces a confirmation modal that appears before starting a new chat session if there are existing messages, preventing accidental loss of chat history. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatBotWindow
participant ConfirmNewChatModal
User->>ChatBotWindow: Click "New Chat"
ChatBotWindow->>ChatBotWindow: Check if messages exist
alt Messages exist
ChatBotWindow->>ConfirmNewChatModal: Open modal
ConfirmNewChatModal->>User: Show confirmation
User->>ConfirmNewChatModal: Click "Erase and start a new chat"
ConfirmNewChatModal->>ChatBotWindow: onConfirm()
ChatBotWindow->>ChatBotWindow: Reset conversation ID and clear messages
else No messages
ChatBotWindow->>ChatBotWindow: Reset conversation ID and clear messages
end
Estimated code review effort2 (~15 minutes) Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "@openshift-assisted/eslint-config" to extend from. Please check that the name of the config is correct. The config "@openshift-assisted/eslint-config" was referenced from the config file in "/libs/chatbot/.eslintrc.cjs". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. libs/chatbot/lib/components/ChatBot/ConfirmNewChatModal.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "@openshift-assisted/eslint-config" to extend from. Please check that the name of the config is correct. The config "@openshift-assisted/eslint-config" was referenced from the config file in "/libs/chatbot/.eslintrc.cjs". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
openshift-ci
bot
added
approved
|
@celdrake the interaction flow looks pretty good. I think just some copy updates is all that's needed for this. Could we add a modal title and then use the following microcopy? Let's also swap positions for your actions so that the primary action is last. Start a new chat? [Cancel] [Erase and start new chat] |
Thanks @asuwebdesign .
|
celdrake
force-pushed
the
MGMT-21223-change-buttons-behaviour
branch
from
00f3856 to
4c4dbeb
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
celdrake
force-pushed
the
MGMT-21223-change-buttons-behaviour
branch
from
4c4dbeb to
77e9269
Compare
|
/cherry-pick releases/v2.43 |
|
@ammont82: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ammont82, celdrake The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx (1)
103-116: Consider using useStateSafely for state management.Based on the retrieved learning about preventing state updates on unmounted components, consider using
useStateSafelyinstead ofReact.useStatefor the modal state to avoid potential memory leaks.- const [isConfirmModalOpen, setIsConfirmModalOpen] = React.useState(false); + const [isConfirmModalOpen, setIsConfirmModalOpen] = useStateSafely(false);Note: This would require importing the
useStateSafelyhook if it's available in your codebase.libs/chatbot/lib/components/ChatBot/ConfirmNewChatModal.tsx (1)
11-17: Consider adding TypeScript interface for props.For better type safety and documentation, consider defining an interface for the component props instead of using inline type annotation.
+interface ConfirmNewChatModalProps { + onConfirm: VoidFunction; + onCancel: VoidFunction; +} + -const ConfirmNewChatModal = ({ - onConfirm, - onCancel, -}: { - onConfirm: VoidFunction; - onCancel: VoidFunction; -}) => { +const ConfirmNewChatModal = ({ onConfirm, onCancel }: ConfirmNewChatModalProps) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
libs/chatbot/lib/components/ChatBot/ChatBot.tsx(0 hunks)libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx(3 hunks)libs/chatbot/lib/components/ChatBot/ConfirmNewChatModal.tsx(1 hunks)
🧠 Learnings (2)
📓 Common learnings
Learnt from: celdrake
PR: openshift-assisted/assisted-installer-ui#3051
File: libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx:196-222
Timestamp: 2025-07-18T12:35:50.945Z
Learning: In the assisted-installer-ui chatbot feedback implementation, the onFeedbackSubmit callback requires access to the messages array to retrieve both the bot response content and the associated user question for the API call, making it necessary to include messages in the useCallback dependency array rather than passing message content as props to avoid duplicating potentially long message data.
libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx (2)
Learnt from: celdrake
PR: #3051
File: libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx:196-222
Timestamp: 2025-07-18T12:35:50.945Z
Learning: In the assisted-installer-ui chatbot feedback implementation, the onFeedbackSubmit callback requires access to the messages array to retrieve both the bot response content and the associated user question for the API call, making it necessary to include messages in the useCallback dependency array rather than passing message content as props to avoid duplicating potentially long message data.
Learnt from: rawagner
PR: #2899
File: libs/ui-lib/lib/ocm/components/clusterWizard/OperatorsSelect.tsx:32-55
Timestamp: 2025-04-14T09:11:08.834Z
Learning: In React components with asynchronous operations, useStateSafely custom hook should be used instead of React.useState to prevent state updates on unmounted components and avoid memory leaks.
💤 Files with no reviewable changes (1)
- libs/chatbot/lib/components/ChatBot/ChatBot.tsx
👮 Files not reviewed due to content moderation or server errors (2)
- libs/chatbot/lib/components/ChatBot/ConfirmNewChatModal.tsx
- libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: celdrake
PR: openshift-assisted/assisted-installer-ui#3051
File: libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx:196-222
Timestamp: 2025-07-18T12:35:50.945Z
Learning: In the assisted-installer-ui chatbot feedback implementation, the onFeedbackSubmit callback requires access to the messages array to retrieve both the bot response content and the associated user question for the API call, making it necessary to include messages in the useCallback dependency array rather than passing message content as props to avoid duplicating potentially long message data.
libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx (2)
Learnt from: celdrake
PR: #3051
File: libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx:196-222
Timestamp: 2025-07-18T12:35:50.945Z
Learning: In the assisted-installer-ui chatbot feedback implementation, the onFeedbackSubmit callback requires access to the messages array to retrieve both the bot response content and the associated user question for the API call, making it necessary to include messages in the useCallback dependency array rather than passing message content as props to avoid duplicating potentially long message data.
Learnt from: rawagner
PR: #2899
File: libs/ui-lib/lib/ocm/components/clusterWizard/OperatorsSelect.tsx:32-55
Timestamp: 2025-04-14T09:11:08.834Z
Learning: In React components with asynchronous operations, useStateSafely custom hook should be used instead of React.useState to prevent state updates on unmounted components and avoid memory leaks.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: translation-files
- GitHub Check: tests
- GitHub Check: unit-tests
- GitHub Check: circular-deps
- GitHub Check: lint
- GitHub Check: format
🔇 Additional comments (17)
libs/chatbot/lib/components/ChatBot/ConfirmNewChatModal.tsx (4)
20-20: LGTM - Modal title aligns with PR feedback.The title "Start a new chat?" matches the suggestion from asuwebdesign in the PR comments.
25-32: Button arrangement correctly implements PR feedback.The button order (Cancel first, then primary action) aligns with asuwebdesign's suggestion. The danger variant appropriately signals the destructive nature of the action.
11-35: LGTM! Clean modal implementation.The component structure and PatternFly integration are well implemented. The props interface is clear, event handlers are properly connected, and the modal configuration follows best practices.
25-32: Button arrangement follows the recommended pattern.The button order (Cancel first, then the primary action) correctly implements the feedback from the PR discussion.
libs/chatbot/lib/components/ChatBot/ChatBotWindow.tsx (13)
29-29: LGTM - Clean import addition.The import statement correctly adds the new modal component.
100-100: LGTM - Appropriate state management.The boolean state for modal visibility follows React patterns and is properly initialized.
103-107: LGTM - Clean separation of concerns.The
startNewChatfunction handles the actual reset logic and properly closes the modal. The function name clearly indicates its purpose.
109-116: LGTM - Smart conditional confirmation logic.The logic correctly shows the confirmation modal only when there are existing messages, providing a smooth UX for new conversations while protecting against accidental data loss.
378-383: LGTM - Proper modal integration.The conditional rendering and callback wiring correctly implements the confirmation flow. The modal is only rendered when needed and properly handles both confirm and cancel actions.
29-29: LGTM! Import follows existing patterns.The import statement is correctly formatted and follows the existing import structure in the file.
100-100: LGTM! Appropriate state management.The new state variable for modal visibility is properly initialized and follows React best practices.
103-116: LGTM! Clean function separation and logic.The refactoring effectively separates the decision logic (whether to show confirmation) from the actual chat clearing logic. The conditional check for existing messages prevents unnecessary confirmation dialogs when there's no conversation to lose.
378-383: LGTM! Proper modal integration.The conditional rendering and prop connections are correctly implemented. The modal is properly integrated into the component lifecycle with appropriate event handlers.
29-29: Import addition looks good.The import for the new ConfirmNewChatModal component is properly placed and follows the existing import structure.
100-100: State management implementation is appropriate.The
isConfirmModalOpenstate variable correctly usesuseStatefor this synchronous modal state management. The retrieved learning aboutuseStateSafelyapplies to asynchronous operations, which isn't the case here.
103-116: Function refactoring implements the confirmation flow correctly.The split between
handleNewChatandstartNewChatproperly implements the confirmation workflow:
handleNewChatconditionally shows the modal only when messages existstartNewChatperforms the actual cleanup and closes the modal- Logic prevents unnecessary confirmation dialogs for empty chats
This addresses the core requirement of preventing accidental chat history loss.
378-383: Modal integration is implemented correctly.The conditional rendering of
ConfirmNewChatModalproperly:
- Shows only when
isConfirmModalOpenis true- Passes
startNewChatas the confirm callback- Passes the state setter to close modal as the cancel callback
The integration follows React best practices and maintains proper state management.
openshift-merge-bot
bot
merged commit 355f781
into
openshift-assisted:master
|
@ammont82: new pull request created: #3066 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
* MGMT-21223: Close chat does not clear existing conversation * MGMT-21223: Add confirmation for starting a new chat
4 participants
buttons-behaviour.mp4
Summary by CodeRabbit
New Features
Behavior Changes