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

UI/notifications #10

Merged
merged 48 commits into from
Jun 15, 2024
Merged

UI/notifications #10

merged 48 commits into from
Jun 15, 2024

Conversation

nriedman
Copy link
Contributor

@nriedman nriedman commented May 22, 2024

Build notification system

♻️ Current situation & Problem

Currently, the home view is empty. Moving forward, the UI for the Home Screen will include notifications, most recent vitals recorded, and a button to start the biweekly survey. To begin, we implemented a notification system that pulls notifications linked to the user's document in Firebase cloud storage, then displays them at the top of the dashboard in the Home Screen.

⚙️ Release Notes

  • Restructured the Home Screen dashboard as a sectioned list including Notifications
  • Implemented a NotificationManager class that attaches a snapshot listener to a query on the currently signed in user's notification collection in Firestore
  • The query retrieves notifications that are younger than 10 days old, and the snapshot listener in the NotificationManager then filters for notifications that haven't been completed yet.
  • The NotificationManager is an environment accessible module that is configured in the ENGAGEDelegate.

📚 Documentation

See inline documentation in NotificationManager.swift and Notification.swift.

✅ Testing

Thorough UI testing ensures that notifications are properly displayed when they are added to the user's backend Firestore, and that UI components such as the "mark complete" button and the "show more" / "show less" buttons work as intended.

Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

@nriedman nriedman requested a review from PSchmiedmayer May 22, 2024 21:36
@PSchmiedmayer
Copy link
Member

@nriedman Feel free to tag me here and drop me a Slack message once we have incorporated the elements we discussed about in the meeting yesterday 🚀

@nriedman
Copy link
Contributor Author

@PSchmiedmayer I've gone ahead and incorporated the final changes we discussed. Ready for review!

Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

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

Nice job with the PR @nriedman and great to see the next advancements for the iOS Home Screen!

I had a lot of comments across the PR. Happy to connect tomorrow to answer any questions that you might have!

In addition, we need to resolve the SwiftLint Errors and should add a small UI test that validates the Notifications or other added functionality before merging the PR.

Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

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

Thank you for all the work in this PR @nriedman!
I fixed some very minor things in a small commit; the overall PR looks great!

It would be great if you can resolve all finished conversations and can create GitHub issues for all remaining elements that might still be open and should be addressed in future smaller or bigger PRs.
After this, feel free to merge the PR 🚀

Copy link

codecov bot commented Jun 14, 2024

Codecov Report

Attention: Patch coverage is 78.40617% with 84 lines in your changes missing coverage. Please review.

Project coverage is 78.27%. Comparing base (adf5392) to head (cbcae79).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #10      +/-   ##
==========================================
- Coverage   79.56%   78.27%   -1.29%     
==========================================
  Files          40       46       +6     
  Lines        1360     1638     +278     
==========================================
+ Hits         1082     1282     +200     
- Misses        278      356      +78     
Files Coverage Δ
ENGAGEHF/Dashboard/Dashboard.swift 100.00% <100.00%> (ø)
...oard/Notifications/Views/NotificationSection.swift 100.00% <100.00%> (ø)
ENGAGEHF/ENGAGEHF.swift 100.00% <ø> (ø)
ENGAGEHF/ENGAGEHFDelegate.swift 96.08% <100.00%> (-0.69%) ⬇️
ENGAGEHF/Home.swift 100.00% <100.00%> (ø)
...HF/Measurement/Views/MeasurementRecordedView.swift 89.48% <100.00%> (ø)
ENGAGEHF/Onboarding/InvitationCodeView.swift 97.37% <ø> (ø)
ENGAGEHF/Onboarding/OnboardingFlow.swift 100.00% <ø> (+2.95%) ⬆️
ENGAGEHF/ReusableElements/ShowMoreButton.swift 100.00% <100.00%> (ø)
...HF/ReusableElements/StudyApplicationListCard.swift 100.00% <100.00%> (ø)
... and 7 more

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update adf5392...cbcae79. Read the comment docs.

@nriedman nriedman merged commit c0a95d5 into main Jun 15, 2024
7 checks passed
@nriedman nriedman deleted the UI/notifications branch June 15, 2024 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants