Conversation
There was a problem hiding this comment.
Tested on Android emulator--looks good! Admittedly I'm not as familiar with the notification system so would probably be best if someone who has more experience with it could look this over too.
Just one small request--I noticed that when you edit your previous reminder settings, the dialog box shows you which buses you selected previously, but the slider always starts at 5 minutes. Could you make sure the dialog "remembers" the slider position (in addition to the checkboxes)?
Thanks!
jumpy-cat
left a comment
jumpy-cat
left a comment
There was a problem hiding this comment.
I tested it against simulated backend issues and it appears to work great! Did find a few small issues that you should address, check the comments I left.
Out of scope for this pr but now that the active list of reminders is fetched after each modification, the reminder widgets could be directly updated and the backend would be able to send less push messages. I've added a note to the reminders tracking issue.
…tion When notification requests are not sent to the backend, a MaizebusOKDialog box is shown instead of a default Dialog one. Additionally, the initial slider position for the reminder threshold is saved after a notification is set.
|
I added the requested changes |
There was a problem hiding this comment.
Everything I commented on is addressed and it still seems to work. Updating the reminder threshold in build is a little iffy but should be fine in practice as fetching reminders from the backend should be slower than loading shared preferences. There is an additional refactor of initState in _StopSheetState, but that looks fine too.
jumpy-cat
left a comment
jumpy-cat
left a comment
There was a problem hiding this comment.
Please fix the lint about using BuildContexts across async gaps, bad stuff such as black screens can happen when you try to use an unmounted context.
| title: Text("Failed!\n${e.toString()}")), | ||
| } on Exception { | ||
| showMaizebusOKDialog( | ||
| contextIn: context, |
3 participants
Description
When setting notification requests for a stop, the app checks to make sure the notification list has the reminders to be added and omits the reminders to be deleted. Otherwise, a popup appears telling the user the notification request could not be sent.
Type of Change
feat)fix)Changes Made
stop_sheet.dartnow displays a different error messagemodifyReminders()inincoming_bus_reminder_service.dartnow checks backendTesting Done
Flutter:
flutter analyzeandflutter formatpassedflutter test)Backend:
Integration:
Checklist
[type](scope): short descriptionprint()/console.log()left in production code