-
Notifications
You must be signed in to change notification settings - Fork 1
update(web): update student settings page design #1007
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
base: master
Are you sure you want to change the base?
Conversation
8c329ef
to
ba49849
Compare
packages/ui/src/locales/ar-eg.json
Outdated
"tutor-settings.topics.selection-dialog.title": "شارك خبراتك لتجذب الطلاب الذين يشاركونك الشغف!", | ||
"tutor-settings.topics.selection-dialog.description": "حدد المجالات التي تبرع فيها أو تهتم بها، مثل الرياضة أو التكنولوجيا أو علم النفس، لنساعدك في تخصيص تجربة تدريسية تناسب اهتمامات طلابك وتزيد من تفاعلهم وحماسهم للتعلم.", | ||
"student-settings.add-topics-button.label": "قم بأضافة الموضوعات المفضلة الخاصة بك", | ||
"student-settings.updated-successfully": "تم تحديث بيانانك بنجاح", |
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.
"تم تحديث بياناتك بنجاح."
topics: [], | ||
career: "", | ||
level: IStudent.EnglishLevel.Beginner, | ||
aim: "", |
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.
The defaults should be the actual student data.
Write mock hooks if you needed to.
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.
1178d02
to
f829bad
Compare
f829bad
to
0a9cbd3
Compare
8aa855b
to
30692e4
Compare
packages/headless/src/student.ts
Outdated
const levelValues = Object.values(IStudent.EnglishLevel).filter( | ||
(value) => typeof value === "number" | ||
); | ||
|
||
function getRandomLevel() { | ||
const randomLevel = sample(levelValues); | ||
return randomLevel as IStudent.EnglishLevel; | ||
} | ||
|
||
export function useStudentMeta(): UseQueryResult<{ | ||
career: string; | ||
level: IStudent.EnglishLevel; | ||
aim: string; | ||
}> { | ||
const find = useCallback(async () => { | ||
return { | ||
career: sample([ | ||
"labels.jobs.student", | ||
"labels.jobs.tutor", | ||
"labels.jobs.doctor", | ||
"labels.jobs.police-officer", | ||
]), | ||
level: getRandomLevel(), | ||
aim: "labels.random-aim", | ||
}; | ||
}, []); | ||
|
||
return useQuery({ | ||
queryFn: find, | ||
queryKey: ["find-student-meta"], | ||
}); | ||
} |
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.
First of all... no need for randomness for these mock functions/hooks. Second, I don't think we need to mock this.
You can just implement it as the mocking part falls on the server responsibility.
export function useFindStudentById(id): UseQueryResult<IStudent.FindApiResponse> {
const api = useApi();
const find = useCallback(async () => {
return api.student.findById(...);
}, [api.student]);
return useQuery({
queryFn: find,
queryKey: ["find-student-by-id"],
});
}
1d51e4f
to
6afcef8
Compare
packages/headless/src/student.ts
Outdated
|
||
return useQuery({ | ||
queryFn: find, | ||
queryKey: ["find-student-by-id"], |
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.
Use QueryKey
.
packages/headless/src/topic.ts
Outdated
|
||
return useMutation({ | ||
mutationFn: update, | ||
mutationKey: ["update-user-topics"], |
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.
Use MutationKey
.
const form = useForm<IForm>({ | ||
defaults: { | ||
topics: studentTopics.data || [], | ||
career: studentData?.jobTitle || intl("labels.jobs.student"), |
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.
or empty string is better. Don't force values or auto-fill user inputs.
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.
6afcef8
to
d1c9746
Compare
d1c9746
to
aabd992
Compare
aabd992
to
50687f2
Compare
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.
Please test the implementation and ensure that it's working as expected.
50687f2
to
133b2f8
Compare
133b2f8
to
dbaff00
Compare
Quality Checklist
Links
Important
Incase you have any valuable knowledge that you think that it is worth writing down, feel free to add it to our handbook. Wounder why we are doing this? check this guide.