Refactor calendar to support dynamic selection type changes #2130
Conversation
| @@ -50,15 +49,13 @@ public struct BPKCalendar<DayAccessoryView: View>: View { | |||
| calendar: Calendar, | |||
| validRange: ClosedRange<Date>, | |||
| initialMonthScroll: MonthScroll? = nil, | |||
| calendarAccessibilityConfiguration: CalendarAccessibilityConfiguration, | |||
There was a problem hiding this comment.
This is now handled as it was before, as part of the binding itself, to not allow the API to lead to invalid states
| let accessibilityProvider: CalendarAccessibilityConfiguration | ||
| let dayDate: Date | ||
| let onSelection: (Date) -> Void | ||
| struct CalendarSelectableCell<Cell: View>: View { |
There was a problem hiding this comment.
nothing to see here, just reverting back to the old implementation, which supports better composition of views among other things
| @@ -30,8 +30,8 @@ struct CalendarMonthGrid< | |||
|
|
|||
| @State private var dayCellHeight: CGFloat = 0 | |||
| @ViewBuilder let dayCell: (Date) -> DayCell | |||
| @ViewBuilder let emptyLeadingDayCell: (EmptyCellInfo) -> EmptyLeadingDayCell | |||
There was a problem hiding this comment.
Empty cell info handled internally, as it's not something of interes outside this particualr view.
| @@ -40,56 +40,74 @@ struct CalendarMonthGrid< | |||
| let firstWeekday = calendar.firstWeekday // Locale-aware first day of the week | |||
| let weekdayOfMonthStart = calendar.component(.weekday, from: monthDate) | |||
| // Calculate the offset based on the first weekday | |||
| let weekdaysOffset = (weekdayOfMonthStart - firstWeekday + daysInAWeek) % daysInAWeek | |||
There was a problem hiding this comment.
reverting these lines to where they were before
| } | ||
|
|
||
| @ViewBuilder | ||
| private func previousEmptyCells(daysFromPreviousMonth: Int) -> some View { |
There was a problem hiding this comment.
Extracted this from the above method to make it easier to read, also introducing the usage of a small internal struct DayCellIdentifiable, as the preiovus range only approach weas leading to mis-reusage of some cells, as they had repeating ids (which was a bug).
Now we can set specific IDs to each cell and make sure they do not repeat across weeks/months in the calendar, fixing the bug where sometimes the cells were not rendering correctly
| @ViewBuilder | ||
| private func currentMonthDayCell(numberOfDaysInMonth: Int) -> some View { | ||
| ForEach(0..<numberOfDaysInMonth, id: \.self) { cellIndex in | ||
| let days = Array(0..<numberOfDaysInMonth) |
There was a problem hiding this comment.
same as above, using the struct rather than a ForEach with a range, that led to bugs before
| @@ -18,23 +18,13 @@ | |||
|
|
|||
| import SwiftUI | |||
|
|
|||
| public struct CalendarAccessibilityConfiguration { | |||
There was a problem hiding this comment.
reverted this to the way it was handled before, reducing duplication and potential invalid states
| } | ||
|
|
||
| func returnMakeCellFunction() -> ((Date) -> CalendarSelectableCell) { | ||
| return { dayDate in | ||
| CalendarSelectableCell( | ||
| selectionType: selectionType, | ||
| calendar: calendar, | ||
| accessibilityProvider: calendarAccessibilityConfiguration, | ||
| dayDate: dayDate, | ||
| onSelection: handleSelection | ||
| ) | ||
| } | ||
|
|
||
| } | ||
|
|
||
| @ViewBuilder func emptyLeadingDayCell(for emptyDayInfo: EmptyCellInfo) -> some View { | ||
| switch selectionType { |
There was a problem hiding this comment.
most this changes are reverting back to the old implementation. with an exception mentioned below
| } else { | ||
| // otherwise we occupy the space with a clear view | ||
| DefaultEmptyCalendarDayCell() | ||
| switch selectionType { |
There was a problem hiding this comment.
the relevant change here (the point of this PR) is that the CalendarContainer is now here, wrapping both the single and range selection calendar month containers, instead of the CalendarContainer being inside the single and range respectively. This is what makes the change of selection types dynamically not re-render the entire calendar, as the only thing that's now re-rendered is the months themselves, and not the entire scrollview.
For context, the problem was that before when you changed selection types of the calendar, it'd re-render the whole thing as this switch was the entire calendar including the scrollviews, now the only thing that is re rendered is the months's grids, WITHIN the container scrollview, making the changes to this vlaue NOT trigger a scroll 'reset' 👍
There was a problem hiding this comment.
So essentially:
now:
Container { <--- ScrollView
Range {
Grid
}
or...
Single {
Grid
}
}
before:
Range {
Container { <--- ScrollView
Grid
}
}
or...
Single {
Container { <--- ScrollView
Grid
}
}
Snapshots were updated. Please verify the changes match the expected layout.
|
|
|
||
| import SwiftUI | ||
|
|
||
| protocol RangeCalendarSelectionHandler { |
There was a problem hiding this comment.
Introducing this protocol so that we can extract the selection handling logic from the Container view and eventually swap it with a new implementation, which is a change that's coming for the range selection calendars, which will be experimented upon.
When the new seleciton logic is implemented, we should only add a new class that conforms to this protocol and, if flag is enabled use one or the other implementaion.
| dateFormatter: accessibilityDateFormatter | ||
| ), | ||
| month: month, | ||
| selectionHandler: DefaultRangeCalendarSelectionHandler( |
There was a problem hiding this comment.
When new one is implemented later on, here we'd have something like:
selectionHandler: flagEnabled ? NewHandler() : DefaultHandler()
|
|
||
| import SwiftUI | ||
|
|
||
| struct SingleCalendarMonthContainer<MonthHeader: View, DayAccessoryView: View>: View { |
There was a problem hiding this comment.
nothng to see here, just reverting
| import SwiftUI | ||
| import Backpack_SwiftUI | ||
|
|
||
| struct CalendarExampleDynamicView: View { |
There was a problem hiding this comment.
created this example to play around the idea of dynamically changing the selection mode of the calendar, which is now better supported wityh this pr
frugoman
deleted the
revert-2128-donburi/DON-1048_m1b_flightsdateselector
branch
Refactoring implementation to revert back to previous approach for composition of views for the BPKCalendar on SwiftUI
this refactor includes changes to support proper dynamic changes over the selection mode of the Calendar, which avoids re-renders of the entire calendar when selection mode is changed dynamically.
Also, the change includes a minor refactor that extracts the code for handling range selections, preparing for a tufure change that will require different handling of this depending on a flag's value
Note: Purposefully marking this as
Minor, as this is reverting to the previous API, which was changed as a Minor change mistakenly. This aims to not break clients that bump minor versions expecting no api changesNew.changes.for.dynamic.calendar.mov