Skip to content
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

Create Notifications Page #63

Merged
merged 9 commits into from
Feb 9, 2025
Merged

Create Notifications Page #63

merged 9 commits into from
Feb 9, 2025

Conversation

HazelYuAhiru
Copy link
Contributor

@HazelYuAhiru HazelYuAhiru commented Feb 5, 2025

Description

Created a notification page for invoices.

Screenshots/Media

image

Issues

Closes #60

@HazelYuAhiru HazelYuAhiru linked an issue Feb 5, 2025 that may be closed by this pull request
6 tasks
@Xire7 Xire7 requested review from theNatePi and jessieh9 February 5, 2025 03:28
@theNatePi
Copy link
Collaborator

theNatePi commented Feb 6, 2025

Good work guys, the page looks great so far!
Here are a couple of things to fix:

  • The near due notifications should only be those in which the event is in the next week
    • Any events with is_sent = True should be ignored
  • Try to match the font to the design
  • Date filtering only works if you also click one of the buttons under right after, however, it should work after a date field is updated. Trying using a date picker input and updating the page when the input values changes
  • Filter should update when changed
  • The Notifications page should go inside of the Navbar, not inside of a flexbox with the Navbar. See the Playgorund.jsx file

Lmk if you guys need any clarifications!
@Xire7 @HazelYuAhiru

Copy link
Collaborator

@theNatePi theNatePi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested

@Xire7 Xire7 requested a review from theNatePi February 7, 2025 18:29
@theNatePi
Copy link
Collaborator

theNatePi commented Feb 9, 2025

Additional modifications:

  • Fixed font. All references to "Inter" needed to be "Inter, sans-serif"
  • Eventually we will need to implement a date picker, but we will leave this out

Copy link
Collaborator

@theNatePi theNatePi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jessieh9 jessieh9 merged commit c814ec4 into main Feb 9, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Notifications Page
4 participants