-
Notifications
You must be signed in to change notification settings - Fork 0
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
fix: dashboard updates from visual enhancements #90
Conversation
Visit the preview URL for this PR (updated for commit 4b07df7): https://communityfridgekw-staging--pr90-hanlin-dashboard-sty-40uvbcww.web.app (expires Tue, 25 Jan 2022 02:50:00 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
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.
This looks good! Looks a lot like the Figma design. I just wanted to show some behaviour that's showing on my laptop that I can't really figure out why:
- Not really vertically aligned (it's aligned in your video though so I'm curious what someone else's computer)
- In the same screenshot, I have no idea why the one time donation shows the ending date of 1969. This was one I made before the branch so maybe that accounts for why? It works for a new one I created.
> | ||
{`${startTimeLocal}-${endTimeLocal}`} | ||
</Text> | ||
{volunteerNeeded && ( |
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.
I was previously taught that booleans in this format should have the !! ahead of it so: !!volunteerNeeded
to account for possibly undefined behaviour - just a nitpick though because I don't know if we will get to this situation.
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 note! However, !!
is usually used to cast a non-boolean variable to a boolean for checking (for instance, a string or a number) --> in our case, since volunteerNeeded
is already of type boolean, we don't need to add it. Additionally, this is a mandatory type (you can see the scheduling model) so it'll always have a value :)
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.
yea just saw that haha woops my bad - thanks for the following up!
default: | ||
return ""; | ||
} | ||
}; |
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.
Definitely nitpick but an extra line here for consistency with the rest of the file?
Brief description. What is this change?
Donor Dashboard Visual Enhancements
Add the updated donation dashboard changes from visual enhancements (P1).
Implementation description. How did you make this change?
Before


After
23722ff6-33ba-4d16-8931-b580f231bd08.mp4
Steps to test
Checklist