⚡ Restore chat state when user is remembered #1333
Merged
Conversation
This file contains 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
|
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the WalkthroughThis update introduces a significant enhancement to the "Remember user" feature, focusing on saving and restoring the chat state across user sessions. It involves changes across multiple files, from adding new utility functions for managing chat state in web storage to updating components for handling transitions and persistence. The primary goal is to ensure a seamless user experience by maintaining chat continuity, with added attention to security and data integrity in the storage mechanism. Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (4)
apps/docs/mint.jsonis excluded by:!**/*.jsonpackages/embeds/js/package.jsonis excluded by:!**/*.jsonpackages/embeds/nextjs/package.jsonis excluded by:!**/*.jsonpackages/embeds/react/package.jsonis excluded by:!**/*.json
Files selected for processing (26)
- apps/builder/src/features/settings/components/GeneralSettingsForm.tsx (2 hunks)
- apps/docs/settings/overview.mdx (1 hunks)
- apps/docs/settings/remember-user.mdx (1 hunks)
- packages/embeds/js/.eslintrc.cjs (1 hunks)
- packages/embeds/js/src/components/Bot.tsx (6 hunks)
- packages/embeds/js/src/components/ConversationContainer/AvatarSideContainer.tsx (2 hunks)
- packages/embeds/js/src/components/ConversationContainer/ChatChunk.tsx (6 hunks)
- packages/embeds/js/src/components/ConversationContainer/ConversationContainer.tsx (4 hunks)
- packages/embeds/js/src/components/InputChatBlock.tsx (6 hunks)
- packages/embeds/js/src/components/bubbles/HostBubble.tsx (1 hunks)
- packages/embeds/js/src/features/blocks/bubbles/audio/components/AudioBubble.tsx (4 hunks)
- packages/embeds/js/src/features/blocks/bubbles/embed/components/CustomEmbedBubble.tsx (4 hunks)
- packages/embeds/js/src/features/blocks/bubbles/embed/components/EmbedBubble.tsx (3 hunks)
- packages/embeds/js/src/features/blocks/bubbles/image/components/ImageBubble.tsx (4 hunks)
- packages/embeds/js/src/features/blocks/bubbles/textBubble/components/TextBubble.tsx (3 hunks)
- packages/embeds/js/src/features/blocks/bubbles/video/components/VideoBubble.tsx (4 hunks)
- packages/embeds/js/src/features/blocks/inputs/buttons/components/Buttons.tsx (2 hunks)
- packages/embeds/js/src/features/blocks/inputs/buttons/components/MultipleChoicesForm.tsx (1 hunks)
- packages/embeds/js/src/features/blocks/inputs/number/components/NumberInput.tsx (1 hunks)
- packages/embeds/js/src/features/bubble/components/Bubble.tsx (7 hunks)
- packages/embeds/js/src/features/popup/components/Popup.tsx (4 hunks)
- packages/embeds/js/src/types.ts (1 hunks)
- packages/embeds/js/src/utils/injectStartProps.ts (1 hunks)
- packages/embeds/js/src/utils/persist.ts (1 hunks)
- packages/embeds/js/src/utils/storage.ts (2 hunks)
- packages/embeds/js/src/window.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/embeds/js/src/features/blocks/inputs/buttons/components/MultipleChoicesForm.tsx
Additional comments: 55
packages/embeds/js/.eslintrc.cjs (1)
- 9-9: Adding the rule
'solid/reactivity': 'off'disables the Solid.js reactivity linting rule. It's crucial to ensure that this decision doesn't negatively impact code quality, especially in a project that leverages Solid.js for its reactive capabilities. Could you provide more context on the rationale behind disabling this rule? It might be beneficial to consider enabling it to catch potential reactivity issues early in the development process.packages/embeds/js/src/types.ts (1)
- 14-14: The addition of the
storagefield to theBotContexttype is a positive step towards achieving the PR's objectives of enhancing user experience through chat state persistence. It's recommended to add documentation or comments explaining the use of this field, especially the scenarios whereundefinedmight be used, to provide clarity for future maintainers and developers.apps/docs/settings/remember-user.mdx (1)
- 6-13: The documentation for the "Remember user" feature is clear and informative, effectively explaining the functionality and the distinction between local and session storage options. To enhance clarity, consider adding examples or use cases that illustrate when one might prefer local storage over session storage and vice versa. This could help users make more informed decisions based on their specific needs.
packages/embeds/js/src/utils/injectStartProps.ts (1)
- 1-1: The adjustments in import statements and the removal of the
eslint-disable solid/reactivitycomment are noted. While these changes likely aim to improve code quality and maintainability, it's important to ensure that the removal of the eslint-disable comment doesn't lead to overlooked reactivity issues in Solid.js components. Please ensure that this change has been thoroughly tested.packages/embeds/js/src/components/ConversationContainer/AvatarSideContainer.tsx (1)
- 5-8: The addition of the
isTransitionDisabledproperty to thePropstype inAvatarSideContaineris a thoughtful enhancement that offers more control over animations. To improve maintainability and clarity for future developers, consider adding documentation or comments explaining the scenarios in which disabling transitions might be desirable.packages/embeds/js/src/features/blocks/bubbles/embed/components/EmbedBubble.tsx (1)
- 10-10: The update making the
onTransitionEndproperty optional in theEmbedBubblecomponent enhances flexibility and customization. To aid future developers and maintainers, it would be beneficial to document the scenarios in which theonTransitionEndcallback is expected to be used and its impact on the component's behavior.packages/embeds/js/src/features/blocks/bubbles/audio/components/AudioBubble.tsx (1)
- 10-10: The modifications to make the
onTransitionEndproperty optional in theAudioBubblecomponent are well-considered, offering greater flexibility in how the component is used. Similar to previous suggestions, documenting theonTransitionEndproperty's purpose and usage scenarios would be beneficial for clarity and maintainability.packages/embeds/js/src/features/blocks/bubbles/embed/components/CustomEmbedBubble.tsx (1)
- 10-10: The update making the
onTransitionEndproperty optional in theCustomEmbedBubblecomponent is a positive change that enhances flexibility. Documenting theonTransitionEndproperty's intended use and scenarios would further improve the component's clarity and maintainability.packages/embeds/js/src/utils/storage.ts (5)
- 1-2: Imports for
InitialChatReplyanddefaultSettingshave been added. Ensure that these imports are used appropriately within the file and that there are no unused imports to maintain code cleanliness.- 4-4: The renaming of
sessionStorageKeytostorageResultIdKeyis a good practice for clarity and better reflects the purpose of the key. Ensure that all references to this key throughout the codebase have been updated accordingly to avoid any broken functionality.Verification successful
The script execution did not find any occurrences of
sessionStorageKeywithin JavaScript files in the codebase, suggesting that the renaming tostorageResultIdKeyhas been successfully reflected throughout. This aligns with the goal of ensuring all references to this key have been updated accordingly.* 32-65: The addition of `getInitialChatReplyFromStorage` and `setInitialChatReplyInStorage` functions is crucial for managing the initial chat reply state. It's important to ensure that error handling is consistent and that any potential exceptions are caught and handled appropriately to avoid breaking the chat application's flow. * 67-89: The functions `setBotOpenedStateInStorage`, `removeBotOpenedStateInStorage`, and `getBotOpenedStateFromStorage` are added to manage the bot's opened state. This is a good approach to track the bot's visibility state across sessions. However, consider adding logging for the catch blocks to help with debugging in case of errors.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for old key usage to ensure it's fully replaced. rg --type js "sessionStorageKey"Length of output: 32
+ console.error("Error managing bot opened state in storage:", error);
- 91-96: The
parseRememberUserStoragefunction simplifies the selection between local and session storage based on settings. This is a clean and efficient way to handle storage preferences. Ensure that the default setting aligns with the application's privacy and persistence requirements.packages/embeds/js/src/components/bubbles/HostBubble.tsx (2)
- 24-24: Making the
onTransitionEndfunction optional in thePropstype is a good practice for flexibility, allowing components to be used in contexts where transition end handling is not necessary. This change enhances the component's reusability.- 28-70: The direct passing of
onTransitionEndandonCompletedfrompropsto the respective components without intermediate functions simplifies the code and reduces unnecessary complexity. This is a good example of the KISS (Keep It Simple, Stupid) principle in action. Ensure that all components receiving these props are properly handling them, especially withonTransitionEndnow being optional.apps/docs/settings/overview.mdx (1)
- 17-17: Linking the "Remember user" setting to a separate page for more details is a user-friendly approach, providing users with a dedicated resource to understand the feature better. Ensure that the linked page is comprehensive and covers all necessary details about the "Remember user" functionality, including how the chat state is saved and restored.
packages/embeds/js/src/features/blocks/inputs/buttons/components/Buttons.tsx (2)
- 10-10: Renaming the
inputIndexprop tochunkIndexin thePropstype declaration improves clarity, as it better describes the purpose of the prop within the context of handling chunks of input. Ensure that all instances where this prop is used have been updated accordingly to maintain consistency.Verification successful
The renaming of the
inputIndexprop tochunkIndexappears to have been thoroughly completed, as no instances of the old prop name were found in the JavaScript files. This suggests that the refactoring was successful and consistent throughout the codebase.* 69-69: The conditional check using `chunkIndex` to determine if a ping animation should be shown is a good use of the renamed prop. This logic ensures that the animation is only applied under specific conditions, enhancing the user interface's dynamic aspects. Ensure that the logic aligns with the intended user experience and that the animation behaves as expected across different scenarios.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for old prop usage to ensure it's fully replaced. rg --type js "inputIndex"Length of output: 25
packages/embeds/js/src/features/blocks/bubbles/textBubble/components/TextBubble.tsx (2)
- 14-14: Making the
onTransitionEndfunction optional in thePropstype increases the component's flexibility, allowing it to be used in various contexts without requiring a transition end handler. This change is a good practice for component design.- 23-31: The conditional initialization of the
isTypingstate based on the presence ofprops.onTransitionEndis a smart way to optimize the component's behavior based on its usage context. This ensures that typing animations are only processed when necessary, potentially improving performance. Ensure that this logic does not inadvertently affect the expected behavior in scenarios whereonTransitionEndis not provided.packages/embeds/js/src/features/blocks/bubbles/image/components/ImageBubble.tsx (2)
- 10-10: Making the
onTransitionEndfunction optional in thePropstype is a thoughtful change that enhances the component's versatility. This allows for more flexible usage of theImageBubblecomponent in different contexts where transition end handling may not be necessary.- 19-33: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [22-58]
The conditional initialization of the
isTypingstate and the dynamic setting of class names based on the presence ofprops.onTransitionEndare well-implemented. These changes ensure that the component behaves appropriately based on whether transition end handling is required, improving the user experience by optimizing the display and animation of images. Verify that the conditional logic does not introduce any unintended side effects in scenarios whereonTransitionEndis not provided.apps/builder/src/features/settings/components/GeneralSettingsForm.tsx (1)
- 85-91: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [88-115]
Updating the description related to remembering the user to emphasize restoring the chat state when the user returns after exiting is a valuable clarification. This change helps set clear expectations for the feature's behavior. Ensure that the description is consistent with the implemented functionality and that it is clearly communicated to users in the UI.
packages/embeds/js/src/features/blocks/bubbles/video/components/VideoBubble.tsx (3)
- 18-18: Making the
onTransitionEndfunction optional in thePropstype for theVideoBubblecomponent is a good practice that increases the component's flexibility. This allows for its use in various contexts without the need for transition end handling, which can be particularly useful for video content that may not always require such functionality.- 15-31: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [26-42]
The conditional initialization of the
isTypingstate based on the presence ofprops.onTransitionEndis a thoughtful approach to optimize the component's behavior. This ensures that typing animations and related transitions are only processed when necessary. It's important to verify that this conditional logic does not affect the expected behavior in scenarios whereonTransitionEndis not provided, ensuring a smooth user experience.
- 49-61: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [52-80]
Conditionally setting the
autoplayattribute of thevideoelement based on the presence ofprops.onTransitionEndis a clever way to control video playback behavior. This ensures that videos do not autoplay in contexts where a transition end event is expected to trigger playback, providing a more controlled and potentially less intrusive user experience. Ensure that this behavior aligns with user expectations and that it is tested across different browsers for consistency.packages/embeds/js/src/features/popup/components/Popup.tsx (5)
- 15-19: The import statements for
getBotOpenedStateFromStorage,removeBotOpenedStateInStorage, andsetBotOpenedStateInStorageare correctly added to manage the bot's opened state using storage utilities. This aligns with the PR's objective to enhance the chat application's user experience by remembering users and their chat states across sessions. Ensure that these utility functions are thoroughly tested, especially for edge cases where storage might be inaccessible (e.g., in private browsing modes).- 43-50: The logic within
onMountto determine if the bot should be opened initially is well-structured and considers multiple conditions:defaultOpen, an ongoing payment process, and the bot's opened state from storage. This comprehensive approach ensures a seamless user experience by restoring the chat state accurately. However, consider adding comments to explain the rationale behind each condition for future maintainability.- 107-107: The call to
removeBotOpenedStateInStoragewithin thecloseBotfunction ensures that the bot's opened state is correctly reset when the bot is closed. This is crucial for maintaining the integrity of the user's session state. It's important to verify that this action does not inadvertently affect the user's experience, especially in scenarios where the user might close and reopen the bot in quick succession.- 114-117: The
handleOnChatStatePersistedfunction is a thoughtful addition, allowing for the execution of a callback when the chat state is persisted, and conditionally callingsetBotOpenedStateInStorageif the state is indeed persisted. This encapsulates the logic for handling chat state persistence neatly. However, ensure that theisPersistedflag accurately reflects the success of the chat state persistence operation, including error handling and potential failure scenarios.- 150-154: The updated
Botcomponent invocation now includes theonChatStatePersistedprop, which is correctly passed thehandleOnChatStatePersistedfunction. This change effectively integrates the chat state persistence functionality into the bot's behavior. It's essential to ensure that theBotcomponent and its props are well-documented, especially regarding how theonChatStatePersistedcallback is expected to be used by the component.packages/embeds/js/src/components/ConversationContainer/ChatChunk.tsx (5)
- 15-20: The renaming of
inputIndextoindexand the addition ofisTransitionDisabledprop are clear and straightforward. These changes align with the PR's objective to enhance chat functionality by potentially disabling transitions for certain chat chunks. This could improve performance or user experience under specific conditions.- 30-32: The initialization of
displayedMessageIndexbased onisTransitionDisabledprop is a logical change. By setting thedisplayedMessageIndexto the length of messages when transitions are disabled, it ensures that all messages are displayed immediately without waiting for transitions. This is a smart way to handle scenarios where animations or transitions are not desired, improving the chat's responsiveness.- 87-87: Passing the
isTransitionDisabledprop to theAvatarSideContainercomponent is a good practice, ensuring that the behavior of child components aligns with the parent's state. This consistency is crucial for maintaining a predictable and coherent user experience across the chat interface.- 114-119: The conditional handling of
onTransitionEndbased onisTransitionDisabledprop is well-implemented. By settingonTransitionEndtoundefinedwhen transitions are disabled, it effectively skips the transition end handling, which is unnecessary in this context. This change optimizes performance by avoiding the execution of transition-related logic when it's not needed.- 131-131: Renaming
inputIndextochunkIndexin theInputChatBlockcomponent call is consistent with the renaming done in the rest of the file. This consistency is important for readability and maintainability of the code. Ensuring that terminology is uniform across related components helps developers understand the codebase more easily.packages/embeds/js/src/features/bubble/components/Bubble.tsx (5)
- 17-21: The addition of storage utility functions (
getBotOpenedStateFromStorage,removeBotOpenedStateInStorage,setBotOpenedStateInStorage) is a significant improvement for managing the bot's open state persistently. This change enhances the user experience by remembering the bot's state across sessions, aligning with the PR's objectives to improve chat functionality and user engagement.- 68-69: Using
getBotOpenedStateFromStorageto determine whether to automatically open the bot on mount is a practical application of the newly added storage functions. This behavior supports the seamless user experience by restoring the bot's previous state, making the chat interaction more intuitive and less disruptive for returning users.- 119-119: The call to
removeBotOpenedStateInStorageupon closing the bot ensures that the bot's open state is accurately reflected in the storage. This cleanup is essential for maintaining the integrity of the bot's state management, preventing potential discrepancies between the actual state and the stored state.- 153-156: The implementation of
handleOnChatStatePersistedis a thoughtful addition, allowing for custom handling of chat state persistence events. This function not only triggers a callback with the persistence result but also callssetBotOpenedStateInStoragewhen the state is persisted, further enhancing the bot's state management capabilities.- 207-207: Passing
handleOnChatStatePersistedas a prop to theBotcomponent is a good practice, ensuring that the bot component can react to chat state persistence events. This integration facilitates a more dynamic and responsive chat experience, allowing for additional actions or feedback based on the persistence outcome.packages/embeds/js/src/components/InputChatBlock.tsx (5)
- 39-39: The introduction of the
persistimport from@/utils/persistis a key change that suggests enhancements in how the component handles state persistence. This import is crucial for implementing the persistence logic that follows, indicating a shift towards more robust and maintainable state management practices.- 53-62: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [46-59]
Renaming
inputIndextochunkIndexand modifying the state initialization foranswerto include persistence settings based on context and chunk index are significant improvements. These changes not only enhance the clarity and consistency of the code but also introduce a more sophisticated approach to managing user input, ensuring that the chat state is preserved across sessions in a secure and efficient manner.
- 74-74: Updating references to
inputIndexto usechunkIndexwithin thecreateEffecthook is a necessary adjustment following the renaming. This consistency is important for ensuring that the logic operates correctly with the updated variable names, maintaining the integrity of the component's functionality.- 108-108: The propagation of the
chunkIndexrenaming to theInputcomponent call within theInputChatBlockcomponent is correctly implemented. This change ensures that all parts of the component are aligned with the new naming convention, contributing to the overall readability and maintainability of the code.- 124-124: The consistent use of
chunkIndexin theInputcomponent's props further demonstrates the thorough application of the renaming throughout the component. This attention to detail is commendable, as it helps prevent potential bugs and confusion that could arise from inconsistent naming.packages/embeds/js/src/components/ConversationContainer/ConversationContainer.tsx (3)
- 35-35: The addition of the
persistimport from@/utils/persistis a foundational change that sets the stage for the persistence-related modifications in this file. This import is essential for enabling the persistence functionality that follows, indicating a strategic enhancement to the component's state management capabilities.- 73-84: Modifying the initialization of
chatChunksto usepersistwith additional configuration is a significant improvement. This change ensures that the chat chunks are persisted across sessions, enhancing the user experience by maintaining chat continuity. The use of a specific key and storage option based on the context is a thoughtful approach to ensuring that the persistence is both effective and contextually appropriate.- 283-289: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [286-300]
Adjusting the
ChatChunkcomponent usage by changing theinputIndexprop toindexand adding theisTransitionDisabledprop based on index comparison is a logical and necessary update. These changes align with the modifications in theChatChunkcomponent and introduce a mechanism to control transitions dynamically, contributing to a more refined and customizable chat experience.packages/embeds/js/src/components/Bot.tsx (6)
- 11-12: The addition of
getInitialChatReplyFromStorageandsetInitialChatReplyInStoragefunctions is crucial for the feature's functionality. Ensure that these functions properly handle edge cases and errors, such as invalid or corrupted data in storage.- 25-26: The import of
defaultSettingsandpersistutilities is a good practice for maintaining consistency and leveraging existing functionality for data persistence. Verify that these utilities are used correctly throughout the file.- 42-42: The addition of
onChatStatePersistedtoBotPropsis a thoughtful way to provide callbacks for the state persistence outcome. Ensure that this callback is invoked appropriately in all relevant scenarios.- 119-148: The logic for storing and retrieving the initial chat replies based on the
rememberUsersettings and theresultIdis well-structured. However, consider adding error handling for potential issues with storage access or data serialization.- 207-216: The conditional logic for determining the storage mechanism based on the
rememberUsersettings is clear and concise. Ensure that the default settings are correctly applied when specific settings are not provided.- 242-247: The use of the
persistutility to manage the progress value with local or session storage is a good approach for enhancing the user experience. Verify that the key used for storage (typebot-${props.context.typebot.id}-progressValue) is unique and does not conflict with other stored data.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #993
Summary by CodeRabbit
New Features
Improvements
onTransitionEndacross multiple bubble components.Refactor
inputIndextochunkIndexorindexin various components for consistency.Bug Fixes