H3126 | Case : Call button in the Chat Widget#413
Open
dvCodeWorld wants to merge 4 commits into
Open
Conversation
|
PR review rate limit exceeded |
❌ Deploy Preview for msg91-chat-widget failed.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughA new ChangesQuickActionsMenu Feature Integration
Dev Server Port Config
Sequence Diagram(s)sequenceDiagram
participant User
participant QuickActionsMenu
participant ChatbotDrawer
participant Redux
participant ParentWindow
rect rgba(100, 149, 237, 0.5)
note over User,QuickActionsMenu: Minimize flow
User->>QuickActionsMenu: click Minimize
QuickActionsMenu->>ChatbotDrawer: onMinimize callback
ChatbotDrawer->>ChatbotDrawer: toggleFullScreen(false) if fullScreen active
ChatbotDrawer->>ParentWindow: postMessage(EXIT_FULL_SCREEN_CHATBOT)
ChatbotDrawer->>Redux: dispatch setDataInDraftReducer({isChatbotMinimized: true})
ChatbotDrawer->>ParentWindow: emitEventToParent(MINIMIZE_CHATBOT)
end
rect rgba(144, 238, 144, 0.5)
note over User,ParentWindow: Fullscreen flow
User->>QuickActionsMenu: click Enter Fullscreen
QuickActionsMenu->>ChatbotDrawer: onToggleFullScreen callback
ChatbotDrawer->>ParentWindow: postMessage(ENTER_FULL_SCREEN_CHATBOT)
ChatbotDrawer->>ChatbotDrawer: setFullScreen(true)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Collaborator
Author
|
/windsurf-review |
|
PR review rate limit exceeded |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/Interface-Chatbot/ChatbotDrawer.tsx`:
- Around line 418-423: The local fullScreen state is initialized to false and
never synced with isFullScreen, creating two separate sources of truth for
fullscreen state. Initialize the fullScreen state with the value from
isFullScreen, and add a useEffect hook that updates the local fullScreen state
whenever the isFullScreen prop/value changes. This will ensure the local state
stays synchronized with isFullScreen and prevent inconsistent menu behavior in
the toggleFullScreen function.
In `@components/Interface-Chatbot/ChatbotHeader.tsx`:
- Around line 617-622: The Expand control is currently implemented as a
non-semantic `<div>` element, which is not keyboard-accessible. Replace the
`<div>` containing the onClick handler for `handleToggleMinimize()` with a
semantic `<button>` element. Move all the existing className styling
(cursor-pointer, p-2, rounded-full, hover:bg-gray-200, transition-colors, icn)
and onClick event handler to the new button element, and keep the Maximize2 icon
as a child of the button. This will make the control properly
keyboard-accessible and screen-reader friendly.
- Around line 626-651: The fullscreen action availability is determined using
isFullScreen in the hasFullScreenAction and hasExitFullScreenAction variables,
but the QuickActionsMenu component receives fullScreen state in its
onToggleFullScreen callback, creating a mismatch. Replace the isFullScreen
references in hasFullScreenAction and hasExitFullScreenAction with fullScreen to
ensure the availability logic is consistent with the state that QuickActionsMenu
component actually uses for displaying the correct action.
In `@components/Interface-Chatbot/QuickActionsMenu.tsx`:
- Around line 100-104: The quick-action buttons in the QuickActionsMenu
component have tabIndex={-1} set, which prevents keyboard users from navigating
and selecting menu actions. Remove the tabIndex={-1} attribute from all
quick-action button elements (referenced at lines 114-116, 128-130, 142-144,
154-156) to make them keyboard reachable. The menu container itself may keep its
tabIndex attributes, but individual action buttons should allow normal tab
navigation by removing the negative tabIndex value.
In `@package.json`:
- Line 6: The dev script in package.json now specifies port 3001, but the README
documentation still references http://localhost:3000, which will cause confusion
for users running the project locally. Update all occurrences of
http://localhost:3000 in the README to http://localhost:3001 to align with the
new dev port configuration defined in the dev script.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8d48695f-9797-4f36-9b1c-2fefdd288bff
📒 Files selected for processing (6)
components/FormComponent.tsxcomponents/Interface-Chatbot/ChatbotDrawer.tsxcomponents/Interface-Chatbot/ChatbotHeader.tsxcomponents/Interface-Chatbot/InterfaceChatbot.csscomponents/Interface-Chatbot/QuickActionsMenu.tsxpackage.json
Comment on lines
+418
to
+423
| const [fullScreen, setFullScreen] = useState(false); | ||
|
|
||
| const toggleFullScreen = (enter: boolean) => { | ||
| if (!window?.parent) return; | ||
| setFullScreen(enter); | ||
| const message = enter |
There was a problem hiding this comment.
Fullscreen has two unsynced sources of truth.
isFullScreen controls availability, while local fullScreen controls icon/label/action. Since local state starts at false and isn’t synced, menu behavior can become inconsistent.
Suggested fix
- const [fullScreen, setFullScreen] = useState(false);
+ const [fullScreen, setFullScreen] = useState(isFullScreen);
+
+ useEffect(() => {
+ setFullScreen(isFullScreen);
+ }, [isFullScreen]);
@@
- const canFullScreen = !isMobileSDK && !isFullScreen;
+ const canFullScreen = !isMobileSDK;Also applies to: 443-457
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/Interface-Chatbot/ChatbotDrawer.tsx` around lines 418 - 423, The
local fullScreen state is initialized to false and never synced with
isFullScreen, creating two separate sources of truth for fullscreen state.
Initialize the fullScreen state with the value from isFullScreen, and add a
useEffect hook that updates the local fullScreen state whenever the isFullScreen
prop/value changes. This will ensure the local state stays synchronized with
isFullScreen and prevent inconsistent menu behavior in the toggleFullScreen
function.
Comment on lines
+617
to
+622
| <div | ||
| className="cursor-pointer p-2 rounded-full hover:bg-gray-200 transition-colors icn" | ||
| onClick={(e) => { e.stopPropagation(); handleToggleMinimize(); }} | ||
| > | ||
| <Maximize2 size={22} style={{ transform: 'rotate(90deg)' }} /> | ||
| </div> |
There was a problem hiding this comment.
Use a semantic button for the new Expand control.
ExpandButton is a clickable <div>, which is not keyboard-accessible by default.
Suggested fix
- <div
+ <button
+ type="button"
className="cursor-pointer p-2 rounded-full hover:bg-gray-200 transition-colors icn"
onClick={(e) => { e.stopPropagation(); handleToggleMinimize(); }}
>
<Maximize2 size={22} style={{ transform: 'rotate(90deg)' }} />
- </div>
+ </button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div | |
| className="cursor-pointer p-2 rounded-full hover:bg-gray-200 transition-colors icn" | |
| onClick={(e) => { e.stopPropagation(); handleToggleMinimize(); }} | |
| > | |
| <Maximize2 size={22} style={{ transform: 'rotate(90deg)' }} /> | |
| </div> | |
| <button | |
| type="button" | |
| className="cursor-pointer p-2 rounded-full hover:bg-gray-200 transition-colors icn" | |
| onClick={(e) => { e.stopPropagation(); handleToggleMinimize(); }} | |
| > | |
| <Maximize2 size={22} style={{ transform: 'rotate(90deg)' }} /> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/Interface-Chatbot/ChatbotHeader.tsx` around lines 617 - 622, The
Expand control is currently implemented as a non-semantic `<div>` element, which
is not keyboard-accessible. Replace the `<div>` containing the onClick handler
for `handleToggleMinimize()` with a semantic `<button>` element. Move all the
existing className styling (cursor-pointer, p-2, rounded-full,
hover:bg-gray-200, transition-colors, icn) and onClick event handler to the new
button element, and keep the Maximize2 icon as a child of the button. This will
make the control properly keyboard-accessible and screen-reader friendly.
Comment on lines
+626
to
+651
| // Determine which quick-action items are available | ||
| const hasMinimizeAction = !!MinimizeButton; | ||
| const hasFullScreenAction = !!ScreenSizeToggleButton && !isFullScreen; | ||
| const hasExitFullScreenAction = !!ScreenSizeToggleButton && isFullScreen; | ||
| const hasCloseAction = !!CloseButton; | ||
| const hasNewConversationAction = !!CreateThreadButton; | ||
|
|
||
| const showQuickActions = ( | ||
| hasMinimizeAction || | ||
| hasFullScreenAction || | ||
| hasExitFullScreenAction || | ||
| hasCloseAction || | ||
| hasNewConversationAction | ||
| ); | ||
|
|
||
| const QuickActionsMenuComponent = useMemo(() => ( | ||
| <QuickActionsMenu | ||
| isChatbotMinimized={isChatbotMinimized} | ||
| fullScreen={fullScreen} | ||
| showMinimize={hasMinimizeAction} | ||
| showFullScreen={hasFullScreenAction || hasExitFullScreenAction} | ||
| showNewConversation={hasNewConversationAction} | ||
| showClose={hasCloseAction} | ||
| onMinimize={handleToggleMinimize} | ||
| onToggleFullScreen={() => toggleFullScreen(!fullScreen)} | ||
| onNewConversation={handleCreateNewSubThread} |
There was a problem hiding this comment.
Quick-actions fullscreen logic can show the wrong action state.
Availability is computed from isFullScreen, but menu label/behavior uses local fullScreen. If those differ, users get incorrect “Expand/Exit full screen” behavior.
Suggested fix
- const hasFullScreenAction = !!ScreenSizeToggleButton && !isFullScreen;
- const hasExitFullScreenAction = !!ScreenSizeToggleButton && isFullScreen;
+ const hasFullScreenAction = !!ScreenSizeToggleButton;
+ const hasExitFullScreenAction = false; // no longer needed separately
@@
- showFullScreen={hasFullScreenAction || hasExitFullScreenAction}
+ showFullScreen={hasFullScreenAction}- const [fullScreen, setFullScreen] = useState(false);
+ const [fullScreen, setFullScreen] = useState(isFullScreen);
+ useEffect(() => setFullScreen(isFullScreen), [isFullScreen]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/Interface-Chatbot/ChatbotHeader.tsx` around lines 626 - 651, The
fullscreen action availability is determined using isFullScreen in the
hasFullScreenAction and hasExitFullScreenAction variables, but the
QuickActionsMenu component receives fullScreen state in its onToggleFullScreen
callback, creating a mismatch. Replace the isFullScreen references in
hasFullScreenAction and hasExitFullScreenAction with fullScreen to ensure the
availability logic is consistent with the state that QuickActionsMenu component
actually uses for displaying the correct action.
Comment on lines
+100
to
+104
| {open && ( | ||
| <div | ||
| role="menu" | ||
| aria-orientation="vertical" | ||
| tabIndex={-1} |
There was a problem hiding this comment.
Menu items are not keyboard reachable.
All quick-action buttons are rendered with tabIndex={-1}, so keyboard users can’t navigate/select actions after opening the menu.
Suggested fix
- <div
- role="menu"
- aria-orientation="vertical"
- tabIndex={-1}
+ <div
+ role="menu"
+ aria-orientation="vertical"
className={
@@
- role="menuitem"
- tabIndex={-1}
+ role="menuitem"
+ tabIndex={0}
@@
- role="menuitem"
- tabIndex={-1}
+ role="menuitem"
+ tabIndex={0}
@@
- role="menuitem"
- tabIndex={-1}
+ role="menuitem"
+ tabIndex={0}
@@
- role="menuitem"
- tabIndex={-1}
+ role="menuitem"
+ tabIndex={0}Also applies to: 114-116, 128-130, 142-144, 154-156
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/Interface-Chatbot/QuickActionsMenu.tsx` around lines 100 - 104,
The quick-action buttons in the QuickActionsMenu component have tabIndex={-1}
set, which prevents keyboard users from navigating and selecting menu actions.
Remove the tabIndex={-1} attribute from all quick-action button elements
(referenced at lines 114-116, 128-130, 142-144, 154-156) to make them keyboard
reachable. The menu container itself may keep its tabIndex attributes, but
individual action buttons should allow normal tab navigation by removing the
negative tabIndex value.
| "private": true, | ||
| "scripts": { | ||
| "dev": "next dev --turbopack", | ||
| "dev": "next dev -p 3001 --turbopack", |
There was a problem hiding this comment.
Update README localhost URL to match the new dev port.
Line 6 pins local dev to port 3001, but README still directs users to http://localhost:3000. Please update the README to avoid onboarding/run confusion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` at line 6, The dev script in package.json now specifies port
3001, but the README documentation still references http://localhost:3000, which
will cause confusion for users running the project locally. Update all
occurrences of http://localhost:3000 in the README to http://localhost:3001 to
align with the new dev port configuration defined in the dev script.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://app.clickup.com/t/3687137/86d2nhwjn
Summary by CodeRabbit
New Features
Bug Fixes
Chores