Conversation
| struct GameView : View { | ||
| var game : Game | ||
| @ObservedObject var viewModel: PastGameViewModel | ||
| @StateObject var calendarViewModel = CalendarViewModel.shared |
There was a problem hiding this comment.
This should be ObservedObject since it's not owned by the view and is a singleton. I don't think this could cause bugs in this specific use case, but it's still better to change it just in case.
| } else { | ||
| DispatchQueue.main.async { | ||
| self.alertTitle = "Game can't be added." | ||
| self.alertMessage = "Can't access to the calendar. The request was denied." |
There was a problem hiding this comment.
"Can't access to the calendar" has a grammar mistake. "Can't access the calendar", or something else.
| } | ||
|
|
||
| let calendarEvent = EKEvent(eventStore: self.eventStore) | ||
| calendarEvent.title = "Cornell vs. \(event.opponent.name) \(event.sex) \(event.sport)" |
There was a problem hiding this comment.
The string here is duplicated from line 26. This can be changed to calendarEvent.title = title
|
|
||
| if let url = URL(string: "calshow:\(event.date.timeIntervalSinceReferenceDate)") { | ||
| UIApplication.shared.open(url) | ||
| } |
There was a problem hiding this comment.
After saving the event, the code sets the alert and immediately opens the Calendar app in the same dispatch block. The UIApplication.shared.open(url) might background the app before SwiftUI has a chance to render the alert. The user will be taken to the Calendar app without seeing the "Game added" confirmation. The calshow: open should either be moved into the alert's dismiss action, or the alert should be removed in favor of just opening the Calendar app.
This is not a huge deal in my opinion. Maybe @Xhether has an opinion on which one to do. I think either the alert or opening the calendar is enough, but I don't mind both as well.
| self.alertTitle = "Game can't be added." | ||
| self.alertMessage = "There was an error adding Cornell vs. \(event.opponent.name) to your calendar." | ||
| self.showAlert = true | ||
| } |
There was a problem hiding this comment.
When the user denies calendar permission, they get a dead-end alert with no way to fix it. Best practice is to include a button that opens UIApplication.openSettingsURLString so the user can grant access in Settings.
2 participants
Next steps: Some of the endpoints for the ticketing links for the away games open the general athletics websites instead of the specific games, which is a detail that would be better if fixed in the future.
(I edited the video that's why the demo is like below)
Score.calendar.mp4