[LMN-20] SwiftUI Calendar component #1818

frugoman · 2023-11-14T17:25:18Z

Initial implementation for the Calendar component purely on SwiftUI

Following: Figma's Calendar

currently supporting:

Single selection
Range selection
Adding accessory action for each month

Remaining features to match UIKit's implementation will be added on a needed-basis, like:

Multiple selection
Select whole month accessory action (working within the component)
Support for adding additional info on each day in calendar (like prices)

leading and trailing day cells

unnecessary elements in Calendar components

SingleCalendarContainer_Previews

selection types in tests.

MBaldo83 · 2023-11-16T08:47:31Z

Backpack-SwiftUI/Calendar/Classes/BPKCalendar.swift

+        self.selectionType = selectionType
+
+        monthHeaderDateFormatter = DateFormatter()
+        monthHeaderDateFormatter.dateFormat = "MMMM yyyy"


Does this date format work for RTL and LTR languages?

It's the same format we used before! So yeah it's the way we expect it to be!

MBaldo83 · 2023-11-16T08:49:35Z

...-SwiftUI/Calendar/Classes/AccessibilityConfigurations/RangeAccessibilityConfigurations.swift

+///     is selected as both the first and second date in the range.
+///   - returnDatePrompt: The prompt provided to assistive technologies informing a user that they should now
+///     select a second date.
+public struct RangeAccessibilityConfigurations {


Should this struct live within the calendar component or be a general Backpack utility? Just asking about which folder this should live in for discoverability purposes. If it's in here, there's less chance of it being re-used elsewhere

It's something that's specific for the Calendar, so living within the Calendar folder is the right place I think. It should not be reused, it's super specific to this!

MBaldo83 · 2023-11-16T08:54:19Z

Backpack-SwiftUI/Tests/Calendar/BPKCalendarTests.swift

+        let calendar = Calendar.current
+        let date = calendar.date(from: .init(year: 2023, month: 11, day: 8))!
+
+        assertSnapshot(


👍 very useful to have all the states tested, and visualised through the snapshot 😄

MBaldo83 · 2023-11-16T08:57:10Z

Backpack-SwiftUI/Calendar/Classes/CalendarBadge.swift

+
+import SwiftUI
+
+struct CalendarBadge: View {


Is it worth having snapshot tests for this as well?

Unless I missed these...

This already appears in the calendar snapshot, I don't think its worth it to have it's own, given also how simple it is!

MBaldo83 · 2023-11-16T09:16:24Z

Backpack-SwiftUI/Calendar/Classes/CalendarMonthHeader.swift

+
+/// CalendarMonthHeader is a view that displays the month name and an optional accessory action.
+/// Also exposes a binding to the currently shown month.
+struct CalendarMonthHeader: View {


Should this have a snapshot test as well, it looks like it contains some layout logic for different states

It's also covered by the other snapshots! I dont think it needs its own!

MBaldo83 · 2023-11-16T09:19:11Z

Backpack-SwiftUI/Calendar/Classes/Core/CalendarContainer.swift

+    @ViewBuilder let monthContent: (_ month: Date) -> MonthContent
+
+    var body: some View {
+        ScrollView {


From what I can tell, you're not doing any cell recycling of the months, I guess that's because there's a limit of 12 months, so it's not necessary, is that correct?

the months themselves are not recycled, but the day cells are. if both the day cells and months are dynamically resolved while scrolling, that produces jumps in the UI while scrolling, because a new month will load but then more cells need to render (the days), pushing content up more.

Later on, when we can support using the Grid component with iOS 16 (only LazyVGrid is available for ios 14 :/ ) we'll be able to switch to recycling months and not the days, which would be preferred!

So we're kinda cornered here by the ios versions we support!

MBaldo83 · 2023-11-16T09:20:34Z

Backpack-SwiftUI/Calendar/Classes/Core/CalendarMonthGrid.swift

+    private let daysInAWeek = 7
+
+    var body: some View {
+        // Sunday is the first day of the week, so we need to offset the days to make Monday the first day


I think you don't need the 1st line of this comment

ahh good catch!

MBaldo83 · 2023-11-16T09:29:30Z

Backpack-SwiftUI/Calendar/Classes/Range/Cells/RangeSelectionCalendarDayCell.swift

+                LowerAndUpperBoundSelectedCell(calendar: calendar, date: date)
+            } else if date == selection.lowerBound {
+                LowerBoundSelectedCell(calendar: calendar, date: date)
+            } else if date == selection.upperBound {
+                UpperBoundSelectedCell(calendar: calendar, date: date)
+            } else {
+                InbetweenSelectionCell(calendar: calendar, date: date)


I'm trying to consider the pros / cons of making individual cells like this versus have a view modifier to apply these various states to a common calendar cell.
I guess the way you have it here with distinct View structs gives some simplicity, and clarity when reading this, but it implies more duplication, eg we have to create a BPKText() view in each cell, and apply mostly the same styling to it each time.
Just wondering if you considered making view modifiers to represent each of these cell types, which could be applied to a common view, or if you think that would end up being too painful / not working?

I considered it yes. I ended up with different views for each day cell type to make them not depend on each other too much and have more flexibility when adding more cell types (for example, the component needs to support (not in this PR) adding prices to the day cells)

So flexibility here is more worth having than some duplication.

Duplication is not really happening much though here (except for maybe the fact it's using a BPKText with with lineLimit=1) and a modifier would be slightly harder to use for this usecase, in general, conditionally adding modifiers should not happen, as re-composing should be the approach taken... aaaaanyways haha the different structs approach is prefered for this case

gert-janvercauteren

I noticed that the day headings use a different format compared to the existing calendar. Should we align Mon / Tue / Wed between both UIKit and SwiftUI?

There's also a bit of funkyness when a selection happens, which leads to a double read out of a cell, it feels like it redraws a large portion of the view when the state changes.

frugoman · 2023-11-20T11:37:13Z

I noticed that the day headings use a different format compared to the existing calendar. Should we align Mon / Tue / Wed between both UIKit and SwiftUI?

There's also a bit of funkyness when a selection happens, which leads to a double read out of a cell, it feels like it redraws a large portion of the view when the state changes.

About showing the months Mon/Tue/Wed or M/T/W, I followed Figma for this, which shows days using only one letter!

on the selection funkyness with VoiceOver, I'm not really sure what's going on? I've debugged every component taking part here and nothing I could find gets re-rendered.

for selection state

… into swiftui-calendar

RangeCalendarContainer

frugoman added 11 commits November 2, 2023 19:10

calendar first pass working

7c86316

some fixes

3132ce4

range extracted approach 1

67f83c6

range extracted approach 2

c58a336

range extracted approach 2.1

f394feb

single selection

5ba874d

simplification

4b99ee7

Refactor CalendarMonthGrid to support empty

69399f4

leading and trailing day cells

Refactor calendar view to improve selection logic

fc12843

Extracting new calendar range and single selection cells.

79e1d26

Add calendar screenshots and update calendar UI

3d30f93

frugoman added minor Non breaking change new-component swiftui labels Nov 14, 2023

Add accessibility configurations and hide

890eac1

unnecessary elements in Calendar components

frugoman marked this pull request as ready for review November 15, 2023 15:52

frugoman added 2 commits November 15, 2023 16:58

Add date formatter to

a811227

SingleCalendarContainer_Previews

Add accessibility configurations for calendar

a42edf0

selection types in tests.

MBaldo83 reviewed Nov 16, 2023

View reviewed changes

Update CalendarMonthGrid.swift

f79cb1a

gert-janvercauteren reviewed Nov 17, 2023

View reviewed changes

Refactor calendar selection type to use a binding

ff068ce

for selection state

frugoman added 3 commits November 20, 2023 18:59

Merge branch 'swiftui-calendar' of github.com:Skyscanner/backpack-ios…

a9a9dc8

… into swiftui-calendar

Merge branch 'main' into swiftui-calendar

36b4d0b

Add CalendarRangeSelectionState enum and update

eb9455a

RangeCalendarContainer

frugoman requested a review from MBaldo83 November 20, 2023 18:04

frugoman added 3 commits November 21, 2023 12:43

Update SwiftUIScreenshots.swift

07930a7

Update Podfile.lock

7d076d2

Update Gemfile.lock

a98aafc

frugoman enabled auto-merge (squash) November 21, 2023 12:37

jonnycaley approved these changes Nov 22, 2023

View reviewed changes

frugoman merged commit fd00953 into main Nov 22, 2023

frugoman deleted the swiftui-calendar branch November 22, 2023 09:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LMN-20] SwiftUI Calendar component #1818

[LMN-20] SwiftUI Calendar component #1818

frugoman commented Nov 14, 2023 •

edited

Loading

MBaldo83 Nov 16, 2023

frugoman Nov 16, 2023

MBaldo83 Nov 16, 2023

frugoman Nov 16, 2023

MBaldo83 Nov 16, 2023

MBaldo83 Nov 16, 2023

MBaldo83 Nov 16, 2023

frugoman Nov 16, 2023

MBaldo83 Nov 16, 2023

frugoman Nov 16, 2023

MBaldo83 Nov 16, 2023

frugoman Nov 16, 2023

MBaldo83 Nov 16, 2023

frugoman Nov 16, 2023

MBaldo83 Nov 16, 2023

frugoman Nov 16, 2023

gert-janvercauteren left a comment

frugoman commented Nov 20, 2023

[LMN-20] SwiftUI Calendar component #1818

[LMN-20] SwiftUI Calendar component #1818

Conversation

frugoman commented Nov 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gert-janvercauteren left a comment

Choose a reason for hiding this comment

frugoman commented Nov 20, 2023

frugoman commented Nov 14, 2023 •

edited

Loading