-
Notifications
You must be signed in to change notification settings - Fork 2
Blackout #1
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
base: master
Are you sure you want to change the base?
Conversation
| selectedIndexPaths.remove(at: index) | ||
| selectedDates.remove(at: index) | ||
| // We want to prevent deselecting ship date | ||
| // delegate?.calendar(self, didDeselectDate: date) |
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.
Maybe we could make this more mergeable with another delegate method:
if delegate?.calendar(self, canDeselectDate: date) ?? false {
delegate?.calendar(self, didDeselectDate: date)
selectedIndexPaths.remove(at: index)
selectedDates.remove(at: index)
}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.
Another "too big of a refactor" where I think the correct generalization would be a SelectionMode enum with an .alwaysOneSelectedDay option. The delegate method rings false since everything CAN be deselected, but only by selecting something else.
| if let delegate = self.delegate, let dateBeingSelected = self.dateFromIndexPath(indexPath) { | ||
| if delegate.calendar(self, canSelectDate: dateBeingSelected) == false { | ||
| dayCell.isWeekend = true | ||
| dayCell.isBlackout = true |
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.
Is there any way to add backwards compatibility for isWeekend?
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 was originally trying to re-purpose isWeekend to give us our blackout behavior, and this is actually un-doing that. isWeekend should be back to normal now, and un-used by our app. We set .marksWeekends to false now.
| public lazy var calendar: Calendar = { | ||
| var gregorian = Calendar(identifier: .gregorian) | ||
| gregorian.timeZone = TimeZone(abbreviation: "UTC")! | ||
| //gregorian.timeZone = TimeZone(abbreviation: "UTC")! |
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.
So the Calendar should always be in the user's current time zone?
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.
Should be in the timezone of the shipping facility, if known, but this fixes an off-by-one error since it handles day-dates as "midnight". There is a lot of clean-up that could improve this, but it'd be a large refactor on the pod.
| return calFlowLayout | ||
| } | ||
|
|
||
| // var startOfMonth = { |
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.
No longer needed? Can a shim be used for backwards compatibility?
| public func setDisplayDate(_ date: Date, animated: Bool = false) { | ||
|
|
||
| guard (date >= startDateCache) && (date <= endDateCache) else { return } | ||
| print("startOfMonthCache = \(startOfMonthCache)") |
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.
stray print statements
| public func isLastMonth() -> Bool { | ||
| var dateComponents = DateComponents() | ||
| dateComponents.month = 1 | ||
| print("isLastMonth") |
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.
stray print statements
|
|
||
| public func isFirstMonth() -> Bool { | ||
| var dateComponents = DateComponents() | ||
| dateComponents.month = -1 |
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.
My favorite month of the year is -1
|
Looks good! Main cleanup is just a few stray comments and print statements. Thoughts on upstreaming this code would be good but not necessary |
Probably a mess, but tried to minimize customizing changes (but there are a few, like commenting out the "de-select" code to turn off that feature).