-
-
Notifications
You must be signed in to change notification settings - Fork 144
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
FEATURE - implement realtime alert functionality #177
FEATURE - implement realtime alert functionality #177
Conversation
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 haven't fully looked at it functionality wise, if it works (and testing with Firebase, so I'm not sure yet on the question you had about that error)
But here's several suggestions to please fix, and I'll take a look again
Great work again!! You're doing awesome!
Basic-Car-Maintenance/Shared/MainView/ViewModels/MainTabViewModel.swift
Outdated
Show resolved
Hide resolved
Basic-Car-Maintenance/Shared/MainView/ViewModels/MainTabViewModel.swift
Outdated
Show resolved
Hide resolved
Basic-Car-Maintenance/Shared/MainView/ViewModels/MainTabViewModel.swift
Outdated
Show resolved
Hide resolved
944aff4
to
2e4d6cd
Compare
2e4d6cd
to
8790dce
Compare
This isn't necessary because it makes it incompatible with dark mode, it's fine that it's blue
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.
Great work!! Note I pushed several changes (so you'll need to do git pull
on your branch)
Mainly just formatting changes here and there.
Added those indexes for alerts
I have a few questions. Otherwise this will be good to merge soon!
And can you make sure that the title doesn't get clipped when it's longer (see screenshot) and make sure in the message when I add \n
for a line break, that it shows up as a line break
Basic-Car-Maintenance/Shared/MainView/ViewModels/MainTabViewModel.swift
Outdated
Show resolved
Hide resolved
e29f2de
to
da673fd
Compare
da673fd
to
73b62ac
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.
The code looks good but I haven't been able to test it successfully with the functionality that I'm wanting.
I'm going to accept it, and look at it again after Hacktoberfest, or put it behind a feature flag for now, thanks for all your work on this!
What it Does
Wire the implemented UI for realtime alert system with the server so that it gets updated each time new alert is arrived
Add functionality to realtime alert system by:
isOn: true
, and if there are viewed alerts previously, append to the query to exclude all alerts that ids match the persisted idsHow I Tested
Notes
Screenshot