-
-
Notifications
You must be signed in to change notification settings - Fork 164
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
Added Notifications with repeating Schedules Solves #55 #57
base: main
Are you sure you want to change the base?
Conversation
Nice work @SalikSayyed! I will review the code soon. In terms of the UI, what about placing the weekday / time part inside a modal that pops up instead of placing it inside the list? You can use |
Co-authored-by: Adrian Carolli <[email protected]>
Co-authored-by: Adrian Carolli <[email protected]>
Co-authored-by: Adrian Carolli <[email protected]>
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.
Done reviewing for now! Good work. Let me know when you make the requested changes and I can review it again. Please update the screenshots once you turn it into a Dialog.
Nice work @SalikSayyed, I left a few more comments and then this should be good! |
Co-authored-by: Adrian Carolli <[email protected]>
Co-authored-by: Adrian Carolli <[email protected]>
Changed to SnackBar. no new package needed |
Hello, @watadarkstar will u please review this ? |
components/ToastSnackBar.tsx
Outdated
interface Props { | ||
message: string | ||
show: boolean | ||
changeShow: React.Dispatch<React.SetStateAction<boolean>> |
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.
nit: call this setShow
@SalikSayyed Sure reviewing it now :) Thank you for reminding me! The UI is a bit odd. The pick time and notify buttons don't feel right. What I recommend:
This would make the UI look more clean 🥇 |
@SalikSayyed Aside from the UI, the code looks good and clean! I look forward to seeing the updated UI when you have a chance to tweak it :) |
@SalikSayyed Please also resolve the conflicts with master :) |
Hey @SalikSayyed can you give me write access to your fork so I can modify stuff if needed? |
|
Yes,That would be great. I added u as collaborator sent invite. Or should I do something else for write access?? @watadarkstar |
@SalikSayyed Its looking better! Nice work. I will jump in and tweak stuff and then get this merged in once I tweak the UI a bit :) |
Description
Build
Used Expo local notifications.
fix #55
Ticket Link
Closes #55
How has this been tested?
Pixel 2 API 29 - Simulator
Screenshots
Checklist