-
Notifications
You must be signed in to change notification settings - Fork 6
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
Tutorial Onboarding #200
base: main
Are you sure you want to change the base?
Tutorial Onboarding #200
Conversation
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.
Overall nice job! Just some clean code issues like extra white space, also add some comments on those bigger contributions of code as it can be confusing to someone who is new looking at it
@@ -39,7 +39,7 @@ export class User { | |||
} | |||
} | |||
|
|||
private async loadUser(): Promise<UserData | null> { | |||
public async loadUser(): Promise<UserData | null> { |
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.
why is this now public?
}; | ||
|
||
checkOnboarding(); | ||
|
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.
delete all this extra white space
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.
remove console statement.
<View style={styles.horizontalLine} /> | ||
<View style={styles.scrollerBackgroundColor}> | ||
{rendering ? <NoteSkeleton /> : renderList(notes)} | ||
|
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.
extra white space
|
||
|
||
<View > | ||
|
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.
add some comments on your code
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.
overall, looks great! The additions are fine, just check if everything works out properly, and cleaning up the code a little bit by removing all the white spaces and also adding more comments into certain sections of the code to define its function.
fontSize: 16, | ||
color: theme.text, | ||
fontWeight: "600", | ||
}, |
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.
Good job on the styles, they look proper and keep it concise, along with adding comments to describe what the styling is doing.
setValue={(callback: (arg0: string) => any) => { | ||
const newValue = callback(value); | ||
setValue(newValue); | ||
handleFilters(newValue); |
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.
items for creating the tutorial looks proper with the everything labeled. add more comments to give better understanding of the functioning specifically
setTooltipVisible(false); | ||
setAddNoteTooltipVisible(true); | ||
}} | ||
> |
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.
add comments to this code to describe and show what you are doing here.
@@ -8903,7 +8903,7 @@ prop-types-exact@^1.2.0: | |||
object.assign "^4.1.5" | |||
reflect.ownkeys "^1.1.4" | |||
|
|||
prop-types@^15.5.7, prop-types@^15.6.2, prop-types@^15.7.2, prop-types@^15.8.1: | |||
prop-types@^15.5.7, prop-types@^15.6.1, prop-types@^15.6.2, prop-types@^15.7.2, prop-types@^15.8.1: |
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.
why are you changing the version down? maybe add a code to show why this is happening
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.
@ademDurakovic Your work is impressive, I just have a few suggestions to make the app even better:
Adding a "Skip Tutorial" button would be helpful for users who don’t want to go through the tutorial. Some users may already know how to use the app.
Including the user account/profile page in the tutorial would make it easier for users to understand and access.
It would also be great to show the dark mode feature in the tutorial for users who prefer it.
Lastly, please create test cases for these changes to make sure everything works smoothly.
Fixes #198
What Got Changed
New widgets were added to help guide a new user throughout the app.
Why it got changed?
This provides a smoother onboarding for our new user's and could clear up some confusion on how to use the app.
How it was changed
IN PROGRESS!
Widgets are displayed with some JSX from a library.
Widgets from a tooltip library have been added but show up on every user log on. Currently the userData type needs to be updated and a new PUT API called Edit_User should be created to help keep track if the user has completed the onboarding, if they have then these widgets won't show.