-
Notifications
You must be signed in to change notification settings - Fork 852
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes: #859 - First time users get Welcome message and introductory v… #3676
base: master
Are you sure you want to change the base?
Conversation
Reviewer's Guide by SourceryThis pull request implements a welcome message and introductory video for first-time users of the SUSI Web Chat application. The changes primarily affect the DialogSection component, adding a new dialog for the welcome message and video, and the MessageSection component, introducing logic to show the welcome dialog for first-time visitors. User journey diagram for first-time user welcome messagejourney
title First-time User Welcome Journey
section Initial Visit
User: Arrives at SUSI Web Chat: 5: User
System: Check if user is first-time visitor: 5: System
System: Display welcome message and video: 5: System
User: Views welcome message and video: 5: User
section Subsequent Visits
User: Arrives at SUSI Web Chat: 5: User
System: Check if user is first-time visitor: 5: System
System: No welcome message displayed: 5: System
User: Proceeds with normal chat usage: 5: User
Updated class diagram for DialogSection componentclassDiagram
class DialogSection {
+bool openLogin
+bool openSignUp
+bool openForgotPassword
+bool openHardwareChange
+bool openThemeChanger
+bool tour
+func onLoginSignUp()
+func handleSignUp()
+func onRequestClose()
+func onRequestCloseTour()
+func onForgotPassword()
+func onSignedUp()
}
class MessageSection {
+bool showLogin
+bool showSignUp
+bool showThemeChanger
+bool openForgotPassword
+bool tour
+func handleCloseTour()
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @isuruAb - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider standardizing indentation across the project in future updates. There's a mix of tabs and spaces which could be unified for better consistency.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟡 Security: 1 issue found
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
<Dialog | ||
className='dialogStyle' | ||
modal={false} | ||
open={this.props.openLogin} | ||
autoScrollBodyContent={true} | ||
bodyStyle={this.props.bodyStyle} | ||
contentStyle={{ width: '35%', minWidth: '300px' }} | ||
onRequestClose={this.props.onRequestClose()}> | ||
<Login {...this.props} | ||
handleForgotPassword={this.props.onForgotPassword()} | ||
handleSignUp={this.props.handleSignUp} /> | ||
<Close style={closingStyle} onTouchTap={this.props.onRequestClose()} /> | ||
</Dialog> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (performance): Consider lazy-loading the tour video for better initial load performance
The new tour Dialog with an embedded YouTube video might impact the initial load time. Consider lazy-loading this component or loading the video on-demand to improve performance.
<React.Suspense fallback={<div>Loading...</div>}>
<Dialog
className='dialogStyle'
modal={false}
open={this.props.openLogin}
autoScrollBodyContent={true}
bodyStyle={this.props.bodyStyle}
contentStyle={{ width: '35%', minWidth: '300px' }}
onRequestClose={this.props.onRequestClose}>
<LazyLogin {...this.props} handleForgotPassword={this.props.onForgotPassword} handleSignUp={this.props.handleSignUp} />
<Close style={closingStyle} onTouchTap={this.props.onRequestClose} />
</Dialog>
</React.Suspense>
autoScrollBodyContent={true} | ||
bodyStyle={this.props.bodyStyle} | ||
contentStyle={{ width: '35%', minWidth: '300px' }} | ||
onRequestClose={this.props.onRequestClose()}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Event handlers are being called immediately instead of passed as references
Throughout the component, event handlers like onRequestClose are being called immediately (e.g., this.props.onRequestClose()) instead of passed as references (this.props.onRequestClose). This can lead to unexpected behavior and performance issues. Consider changing these to pass the function references directly.
import Cookies from 'universal-cookie'; | ||
|
||
|
||
const cookies=new Cookies(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚨 suggestion (security): Consider implications of using cookies for user privacy and data persistence
The introduction of cookies should be carefully considered in terms of user privacy and data persistence. Ensure that you're complying with relevant data protection regulations and that users are informed about the use of cookies if necessary.
const cookies=new Cookies(); | |
const cookies = new Cookies({ | |
secure: true, | |
sameSite: 'strict' | |
}); |
…ideo
Fixes #[Add issue number here. If you do not solve the issue entirely, please change the message e.g. "First steps for issue" #IssueNumber]
Changes: [Add here what changes were made in this pull request and if possible provide links showcasing the changes.]
Demo Link: https://pr-[ADD_PULL_REQUEST_NUMBER_HERE]-fossasia-susi-web-chat.surge.sh
Screenshots of the change: [Add screenshots depicting the changes.]
Summary by Sourcery
Add a welcome dialog with an introductory video for first-time users in the SUSI Web Chat application, utilizing cookies to track user visits and ensure the dialog is only shown to new users.
New Features:
Enhancements: