feat: ui ux enhancements, websocket logic for objectives, action list, duration - #216
Conversation
…ase, improving clarity in button action handling.
…hini-app-frontend into feature/ip-socket
…ebSocket state handling and confirmation action clarity. Remove commented-out code in ShikshalokamVoiceChat for cleaner implementation.
…ove error management and user experience. Introduce separate functions for yes and no button actions, ensuring chat history is cleaned correctly before reload.
…ip-socket Feature/ip socket
…connect logic. Enhance ShikshalokamVoiceChat by adding homepage visibility condition and updating WebSocket retry attempts from environment variable.
…ip-socket Update useChatWebhook to initialize reconnectCount at 1 and adjust re…
…tConnected and update WebSocket connection logic to handle cases when the socket is not connected.
…ip-socket Refactor ShikshalokamVoiceChat to rename isFreshConnection to isSocke…
… integrating chat history management. Update createMessage function to include 'received' status and improve chat history filtering logic. Refactor imports for better organization.
…implify the mapping of messages. Remove unnecessary slice operation and directly map chat history for improved performance.
…ip-socket Enhance ShikshalokamChat and ShikshalokamVoiceBasedChat components by…
…ntend into feat/ui-ux-enh
|
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 You can disable this status message by setting the WalkthroughThis PR integrates WebSocket-based chat flows into the AI creation journey across three pages (duration selection, objective selection, action items), adding new routes, chat history state management, and chat-driven UI components while preserving existing functionality. Changes
Sequence DiagramsequenceDiagram
participant User as User/UI
participant Component as AI Creation Page<br/>(WeeksSelection,<br/>SelectObjective,<br/>ActionItems)
participant WebSocket as WebSocket<br/>Connection
participant Server as Backend<br/>Server
participant Store as AI Creation<br/>Store
User->>Component: Mount component / Click action
Component->>WebSocket: Initialize WebSocket connection<br/>(sessionId, accessToken)
WebSocket->>Server: Connect & establish session
Server->>WebSocket: Connection established
WebSocket->>Component: onOpen event
Component->>Server: Authenticate session<br/>(via WebSocket message)
Server->>WebSocket: Authentication ACK
User->>Component: Send chat message<br/>(handleSendMessage)
Component->>WebSocket: Send message via socket
WebSocket->>Server: Transmit user message
Server->>Server: Process & generate response
Server->>WebSocket: Send bot response
WebSocket->>Component: onMessage event
Component->>Store: Update chat history<br/>(setDurationChatHistory, etc.)
Component->>Component: Re-render with<br/>updated chat UI
Component->>User: Display bot response<br/>& chat history
Component->>Server: Persist chat to DB<br/>(onClose / completion)
Server->>Server: Store interaction log
Server->>WebSocket: Acknowledgement
WebSocket->>Component: Connection closed cleanly
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/pages/ShikshalokamVoiceChat/voice-chat.js (3)
67-69: Reconsider hardcoded wss:// protocol and unresolved TODO.The TODO comment suggests this code was intentionally left in an incomplete state. The hardcoded
wss://protocol breaks local/development environments that usews://protocol. This needs either a conditional check or clarification on whether this environment is HTTPS-only.🔎 Suggested conditional protocol fix:
- // TODO: After testing, revert this to the original code - // const wss_protocol = window.location.protocol === "https:" ? "wss://" : "ws://" - const wss_protocol = "wss://" + const wss_protocol = window.location.protocol === "https:" ? "wss://" : "ws://"
1237-1237: Fix redundant condition logic.The condition
if (accessToken || accessToken)is always true when accessToken exists. This is likely a copy-paste error. Verify the intended logic—it should probably be checking a different variable or using&&(logical AND) instead.🔎 Likely intended fix:
- if (accessToken || accessToken) { + if (accessToken) {
643-643: Use strict equality (===) instead of loose equality (==).The comparison
chat_history.filter(chat => chat.source === "user").length == 1uses loose equality, which can lead to unexpected type coercion. Use strict equality===for consistency and to avoid potential bugs.🔎 Apply this diff:
- if (chat_history.filter(chat => chat.source === "user").length == 1) { + if (chat_history.filter(chat => chat.source === "user").length === 1) {src/store/slices/aiCreationData/state.js (1)
86-86: Pre-existing bug:setChunkshas incorrect double arrow function syntax.This line has a curried function that will not work as expected.
setChunksreturns a function instead of actually setting the chunks.🔎 Apply this diff to fix:
- setChunks: () => chunks => set({ chunks }), + setChunks: chunks => set({ chunks }),
🧹 Nitpick comments (14)
src/pages/ai-creation/pages/shikshalokam-mitra/stylesheet/shikshaChatStyle.css (1)
602-605: Consider removing the redundantjustify-contentproperty.Since
display: noneremoves the element from the layout flow, thejustify-content: flex-end;property has no effect and can be removed to keep the CSS clean.Additionally, while the
!importantflag ensures the element stays hidden, consider whether this level of specificity is necessary or if it indicates a specificity conflict that could be resolved through better CSS architecture.🔎 Suggested simplification
.div37 { - justify-content: flex-end; display: none !important; }src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/components/action-items/ActionItemsSwiper.jsx (1)
57-57: Consider click handler placement for accessibility.The
onClickhandler is on the containerdivwhich wraps an interactivebuttonelement. This can cause:
- Click events bubbling when users click the button
- Accessibility concerns since the outer div is not semantically interactive
If the entire card should be clickable, consider using a
<button>or addingrole="button"andtabIndex={0}with keyboard handlers. Alternatively, if only specific interactions are needed, moveonClickto a more targeted element.src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/components/ChatWindow.jsx (2)
26-36: Consider using a constant for magic number and simplifying ternaries.
- Page number
4should be a named constant for clarity- Ternary expressions
x ? false : truecan be simplified to!x🔎 Suggested simplification:
+const WEEKS_SELECTION_PAGE = 4; + const getShowLoadingChat = (indexNumber) => { - const isWeeksSelectionSection = page && page === 4; + const isWeeksSelectionSection = page === WEEKS_SELECTION_PAGE; let showLoader = true; if(isDefineChallengeSection) { - showLoader = objectiveList?.length > 0 ? false : true; + showLoader = !objectiveList?.length; } else if(isWeeksSelectionSection) { - showLoader = selectedWeek ? false : true + showLoader = !selectedWeek; }
147-147: Remove unnecessary fragment wrapper.The fragment
<><LoadingChat /></>serves no purpose here.🔎 Apply this diff:
- {getShowLoadingChat(i) && <><LoadingChat /></>} + {getShowLoadingChat(i) && <LoadingChat />}src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/WeeksSelection.jsx (3)
54-54: Remove debug console.log before merging.Debug logging should be removed from production code.
🔎 Apply this diff:
- console.log({localChatHistory})
79-80: Empty WebSocket close handler may hide connection issues.Consider adding logging or state updates to indicate connection status to the user when the WebSocket closes unexpectedly.
239-246: Remove commented-out Slider code.Commented-out code should be removed rather than left in the codebase. If this functionality might be needed, it can be retrieved from version control history.
src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/SelectObjective.jsx (3)
156-156: Remove debug console.log before merging.🔎 Apply this diff:
- console.log({transformedSource})
383-383: Remove debug console.log before merging.🔎 Apply this diff:
- console.log("coming here final text", finalText)
500-501: Add consistent spacing after ternary operators.🔎 Apply this diff for readability:
- const beforeObjectiveHistory = separatorIndex !== -1 ?objectiveChatHistory?.slice(0, separatorIndex) : [] - const afterObjectiveHistory = separatorIndex !== -1 ?objectiveChatHistory?.slice(separatorIndex + 1) : objectiveChatHistory; + const beforeObjectiveHistory = separatorIndex !== -1 ? objectiveChatHistory?.slice(0, separatorIndex) : [] + const afterObjectiveHistory = separatorIndex !== -1 ? objectiveChatHistory?.slice(separatorIndex + 1) : objectiveChatHistory;src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/ActionItems.jsx (4)
407-407: Remove debug console.log before merging.🔎 Apply this diff:
- console.log({action_to_store})
501-501: Remove debug console.log before merging.🔎 Apply this diff:
- console.log({hasClickedOnAddmore, wantsToMoveForward, actionList, isLoading, isSelectActionItems})
575-587: Remove large commented-out code block.This commented-out code references undefined variables (
objectiveList,handleObjectiveClick,selectedObjective, etc.) that don't exist in this component. It appears to be copy-pasted from SelectObjective.jsx and should be removed.
497-498: Add consistent spacing after ternary operators.🔎 Apply this diff:
- const beforeActionListHistory = separatorIndex !== -1 ?actionListChatHistory?.slice(0, separatorIndex) : [] - const afterActionListHistory = separatorIndex !== -1 ?actionListChatHistory?.slice(separatorIndex + 1) : actionListChatHistory; + const beforeActionListHistory = separatorIndex !== -1 ? actionListChatHistory?.slice(0, separatorIndex) : [] + const afterActionListHistory = separatorIndex !== -1 ? actionListChatHistory?.slice(separatorIndex + 1) : actionListChatHistory;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
public/locales/en/ai_creation_translation.json(2 hunks)src/configure.js(3 hunks)src/hooks/useChatWebhook.js(3 hunks)src/pages/ShikshalokamVoiceChat/enum.js(1 hunks)src/pages/ShikshalokamVoiceChat/voice-chat.js(1 hunks)src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/ActionItems.jsx(15 hunks)src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/DefineChallenge.jsx(1 hunks)src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/SelectObjective.jsx(10 hunks)src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/WeeksSelection.jsx(5 hunks)src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/components/ChatWindow.jsx(3 hunks)src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/components/SuggestOrAddCta.jsx(1 hunks)src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/components/action-items/ActionItemsList.jsx(2 hunks)src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/components/action-items/ActionItemsSwiper.jsx(3 hunks)src/pages/ai-creation/pages/shikshalokam-mitra/stylesheet/shikshaChatStyle.css(1 hunks)src/store/slices/aiCreationData/state.js(2 hunks)src/utils/helpers.js(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/utils/helpers.js (3)
src/pages/ShikshalokamVoiceChat/enum.js (2)
sessionFlowName(32-44)sessionFlowName(32-44)src/configure.js (2)
bot_websocket(38-48)bot_websocket(38-48)src/components/LanguageSelectionGrid.jsx (1)
currentFlow(48-48)
src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/components/action-items/ActionItemsSwiper.jsx (1)
src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/ActionItems.jsx (4)
selectedIndex(65-65)swipeDirection(66-66)actionList(61-61)actionList(653-653)
src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/components/ChatWindow.jsx (5)
src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/SelectObjective.jsx (1)
objectiveList(56-56)src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/ActionItems.jsx (1)
useAICreationSessionStore(108-108)src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/DefineChallenge.jsx (2)
hasStartedListening(74-74)chatHistory(65-67)src/pages/ai-creation/pages/shikshalokam-mitra/MainPage.jsx (1)
chatHistory(49-51)src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/components/LoadingChat.jsx (1)
LoadingChat(1-8)
src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/ActionItems.jsx (2)
src/pages/ai-creation/utils/mitra-chat.js (2)
transformActionListSources(92-163)transformActionListSources(92-163)src/pages/ai-creation/constants/mitra.constants.js (2)
CONVERSATION_USER_TYPES(7-10)CONVERSATION_USER_TYPES(7-10)
src/store/slices/aiCreationData/state.js (3)
src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/WeeksSelection.jsx (1)
durationChatHistory(56-58)src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/SelectObjective.jsx (1)
objectiveChatHistory(80-82)src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/ActionItems.jsx (1)
actionListChatHistory(95-97)
🔇 Additional comments (8)
public/locales/en/ai_creation_translation.json (1)
65-66: LGTM!The new localization keys for
goBacknavigation andenterObjectivePlaceholderare well-structured and consistent with the existing naming conventions.Also applies to: 80-81
src/pages/ShikshalokamVoiceChat/enum.js (1)
43-43: LGTM!The new
Creationenum member follows the existing naming conventions and integrates correctly with the WebSocket configuration inconfigure.jsand the mapping inhelpers.js.src/utils/helpers.js (1)
74-74: LGTM!The new mapping for
sessionFlowName.Creationtobot_websocket.creationcorrectly integrates the Creation flow with the WebSocket configuration.src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/components/action-items/ActionItemsList.jsx (1)
18-18: LGTM!The
handleActionListClickprop is correctly destructured and forwarded toActionItemsSwiper.Also applies to: 44-44
src/configure.js (1)
32-36: LGTM!New route and WebSocket endpoint configurations follow the existing patterns and align with the WebSocket-based chat flow enhancements described in the PR.
Also applies to: 46-47
src/pages/ai-creation/pages/shikshalokam-mitra/mitra-pages/DefineChallenge.jsx (1)
1129-1129: Verify intentional hardcoding ofisDefineChallengeSection={true}in ChatWindow.When
isDefineChallengeSection=falseis passed from the parent (MainPage), DefineChallenge enters its non-welcome branch where ChatWindow receives a hardcodedtrue, while ChatBox rendering remains conditional on the parent's actual prop value. This creates inconsistency: ChatWindow behaves as if in the challenge section regardless of parent intent, while ChatBox and the container layout respect the parent prop. Confirm whether this hardcoding is intentional design or an oversight.src/store/slices/aiCreationData/state.js (2)
31-34: LGTM!The new state fields (
durationChatHistory,objectiveChatHistory,actionListChatHistory,prevObjective) follow the existing patterns and are properly initialized with appropriate default values.
98-108: LGTM!The new getters and setters follow the established patterns in this store and are consistent with other accessors.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.