From b62b69964ae398f312ac2c17e6b0b2353ec86399 Mon Sep 17 00:00:00 2001 From: Bryan Keller Date: Thu, 25 Jan 2024 17:43:54 -0800 Subject: [PATCH 1/3] Update ItemViewReuseManager and tests --- Sources/Internal/ItemViewReuseManager.swift | 8 +- Tests/ItemViewReuseManagerTests.swift | 135 +++++++++++++++++++- 2 files changed, 137 insertions(+), 6 deletions(-) diff --git a/Sources/Internal/ItemViewReuseManager.swift b/Sources/Internal/ItemViewReuseManager.swift index adccf8c..4996989 100644 --- a/Sources/Internal/ItemViewReuseManager.swift +++ b/Sources/Internal/ItemViewReuseManager.swift @@ -25,6 +25,7 @@ final class ItemViewReuseManager { func viewsForVisibleItems( _ visibleItems: Set, + recycleOldViews: Bool, viewHandler: ( ItemView, VisibleItem, @@ -56,6 +57,7 @@ final class ItemViewReuseManager { let context = reusedViewContext( for: visibleItem, + recycleOldViews: recycleOldViews, unusedPreviouslyVisibleItems: &visibleItemsDifference) viewHandler( context.view, @@ -76,6 +78,7 @@ final class ItemViewReuseManager { private func reusedViewContext( for visibleItem: VisibleItem, + recycleOldViews: Bool, unusedPreviouslyVisibleItems: inout Set) -> ReusedViewContext { @@ -103,7 +106,7 @@ final class ItemViewReuseManager { visibleItemsForItemViewDifferentiators[differentiator]?.remove(visibleItem) viewsForVisibleItems.removeValue(forKey: visibleItem) } else { - if let previouslyVisibleItem = unusedPreviouslyVisibleItems.first { + if recycleOldViews, let previouslyVisibleItem = unusedPreviouslyVisibleItems.first { // An unused, previously-visible item is available, so reuse it. guard let previousView = viewsForVisibleItems[previouslyVisibleItem] else { @@ -122,7 +125,8 @@ final class ItemViewReuseManager { visibleItemsForItemViewDifferentiators[differentiator]?.remove(previouslyVisibleItem) viewsForVisibleItems.removeValue(forKey: previouslyVisibleItem) } else { - // No previously-visible item is available for reuse, so create a new view. + // No previously-visible item is available for reuse (or view recycling is disabled), so + // create a new view. view = ItemView(initialCalendarItemModel: visibleItem.calendarItemModel) previousBackingVisibleItem = nil isReusedViewSameAsPreviousView = false diff --git a/Tests/ItemViewReuseManagerTests.swift b/Tests/ItemViewReuseManagerTests.swift index ae6546f..d95aeb9 100644 --- a/Tests/ItemViewReuseManagerTests.swift +++ b/Tests/ItemViewReuseManagerTests.swift @@ -58,6 +58,7 @@ final class ItemViewReuseManagerTests: XCTestCase { reuseManager.viewsForVisibleItems( visibleItems, + recycleOldViews: true, viewHandler: { _, _, previousBackingItem, isReusedViewSameAsPreviousView in XCTAssert( previousBackingItem == nil, @@ -101,11 +102,15 @@ final class ItemViewReuseManagerTests: XCTestCase { let subsequentVisibleItems = initialVisibleItems // Populate the reuse manager with the initial visible items - reuseManager.viewsForVisibleItems(initialVisibleItems, viewHandler: { _, _, _, _ in }) + reuseManager.viewsForVisibleItems( + initialVisibleItems, + recycleOldViews: true, + viewHandler: { _, _, _, _ in }) // Ensure all views are reused by using the exact same previous views reuseManager.viewsForVisibleItems( subsequentVisibleItems, + recycleOldViews: true, viewHandler: { _, item, previousBackingItem, isReusedViewSameAsPreviousView in XCTAssert( item == previousBackingItem, @@ -179,11 +184,15 @@ final class ItemViewReuseManagerTests: XCTestCase { ] // Populate the reuse manager with the initial visible items - reuseManager.viewsForVisibleItems(initialVisibleItems, viewHandler: { _, _, _, _ in }) + reuseManager.viewsForVisibleItems( + initialVisibleItems, + recycleOldViews: true, + viewHandler: { _, _, _, _ in }) // Ensure all views are reused given the subsequent visible items reuseManager.viewsForVisibleItems( subsequentVisibleItems, + recycleOldViews: true, viewHandler: { _, item, previousBackingItem, _ in XCTAssert( item.calendarItemModel._itemViewDifferentiator == previousBackingItem?.calendarItemModel._itemViewDifferentiator, @@ -270,11 +279,15 @@ final class ItemViewReuseManagerTests: XCTestCase { ] // Populate the reuse manager with the initial visible items - reuseManager.viewsForVisibleItems(initialVisibleItems, viewHandler: { _, _, _, _ in }) + reuseManager.viewsForVisibleItems( + initialVisibleItems, + recycleOldViews: true, + viewHandler: { _, _, _, _ in }) // Ensure the correct subset of views are reused given the subsequent visible items reuseManager.viewsForVisibleItems( subsequentVisibleItems, + recycleOldViews: true, viewHandler: { _, item, previousBackingItem, isReusedViewSameAsPreviousView in guard let itemModel = item.calendarItemModel as? MockCalendarItemModel else { preconditionFailure( @@ -408,13 +421,17 @@ final class ItemViewReuseManagerTests: XCTestCase { ] // Populate the reuse manager with the initial visible items - reuseManager.viewsForVisibleItems(initialVisibleItems, viewHandler: { _, _, _, _ in }) + reuseManager.viewsForVisibleItems( + initialVisibleItems, + recycleOldViews: true, + viewHandler: { _, _, _, _ in }) // Ensure the correct subset of views are reused given the subsequent visible items var reuseCountsForDifferentiators = [_CalendarItemViewDifferentiator: Int]() var newViewCountsForDifferentiators = [_CalendarItemViewDifferentiator: Int]() reuseManager.viewsForVisibleItems( subsequentVisibleItems, + recycleOldViews: true, viewHandler: { _, item, previousBackingItem, _ in if previousBackingItem != nil { let reuseCount = (reuseCountsForDifferentiators[item.calendarItemModel._itemViewDifferentiator] ?? 0) + 1 @@ -444,6 +461,116 @@ final class ItemViewReuseManagerTests: XCTestCase { "The number of new view creations does not match the expected number of new view creations.") } + func testDisablingViewRecycling() { + let initialVisibleItems: Set = [ + .init( + calendarItemModel: MockCalendarItemModel.variant0, + itemType: .layoutItemType( + .monthHeader(Month(era: 1, year: 2020, month: 01, isInGregorianCalendar: true))), + frame: .zero), + .init( + calendarItemModel: MockCalendarItemModel.variant0, + itemType: .layoutItemType( + .monthHeader(Month(era: 1, year: 2020, month: 02, isInGregorianCalendar: true))), + frame: .zero), + .init( + calendarItemModel: MockCalendarItemModel.variant1, + itemType: .layoutItemType( + .day( + Day( + month: Month(era: 1, year: 2020, month: 01, isInGregorianCalendar: true), + day: 01))), + frame: .zero), + .init( + calendarItemModel: MockCalendarItemModel.variant1, + itemType: .layoutItemType( + .day( + Day( + month: Month(era: 1, year: 2020, month: 02, isInGregorianCalendar: true), + day: 01))), + frame: .zero), + .init( + calendarItemModel: MockCalendarItemModel.variant2, + itemType: .layoutItemType( + .day( + Day( + month: Month(era: 1, year: 2020, month: 02, isInGregorianCalendar: true), + day: 01))), + frame: .zero), + .init( + calendarItemModel: MockCalendarItemModel.variant3, + itemType: .layoutItemType( + .day( + Day( + month: Month(era: 1, year: 2020, month: 02, isInGregorianCalendar: true), + day: 01))), + frame: .zero), + ] + + let subsequentVisibleItems: Set = [ + .init( + calendarItemModel: MockCalendarItemModel.variant1, + itemType: .layoutItemType( + .day( + Day( + month: Month(era: 1, year: 2020, month: 05, isInGregorianCalendar: true), + day: 01))), + frame: .zero), + .init( + calendarItemModel: MockCalendarItemModel.variant3, + itemType: .layoutItemType( + .day( + Day( + month: Month(era: 1, year: 2020, month: 05, isInGregorianCalendar: true), + day: 01))), + frame: .zero), + .init( + calendarItemModel: MockCalendarItemModel.variant4, + itemType: .layoutItemType( + .monthHeader(Month(era: 1, year: 2020, month: 04, isInGregorianCalendar: true))), + frame: .zero), + .init( + calendarItemModel: MockCalendarItemModel.variant5, + itemType: .layoutItemType( + .monthHeader(Month(era: 1, year: 2020, month: 05, isInGregorianCalendar: true))), + frame: .zero), + ] + + // Populate the reuse manager with the initial visible items + reuseManager.viewsForVisibleItems( + initialVisibleItems, + recycleOldViews: false, + viewHandler: { _, _, _, _ in }) + + // Ensure the correct subset of views are reused given the subsequent visible items + reuseManager.viewsForVisibleItems( + subsequentVisibleItems, + recycleOldViews: false, + viewHandler: { _, item, previousBackingItem, isReusedViewSameAsPreviousView in + guard let itemModel = item.calendarItemModel as? MockCalendarItemModel else { + preconditionFailure( + "Failed to convert the calendar item model to an instance of MockCalendarItemModel.") + } + + switch itemModel { + case .variant1, .variant3: + XCTAssert( + previousBackingItem == nil, + "Previous backing item should be nil since view recycling is disabled.") + XCTAssert( + !isReusedViewSameAsPreviousView, + "isReusedViewSameAsPreviousView should be false when a different view was reused.") + default: + XCTAssert( + previousBackingItem == nil, + "Previous backing item should be nil since there are no views to reuse.") + XCTAssert( + !isReusedViewSameAsPreviousView, + "isReusedViewSameAsPreviousView should be false when a different view was reused.") + } + }) + } + // MARK: Private // swiftlint:disable:next implicitly_unwrapped_optional From 6aecc2811b547957a2021585aa35c20a211d7e4c Mon Sep 17 00:00:00 2001 From: Bryan Keller Date: Thu, 25 Jan 2024 17:59:16 -0800 Subject: [PATCH 2/3] Simplify accessibility implementation in CalendarView PR feedback --- CHANGELOG.md | 6 + Sources/Internal/ItemViewReuseManager.swift | 8 +- .../SubviewInsertionIndexTracker.swift | 124 ++++++++++-------- Sources/Public/CalendarView.swift | 74 +++++------ Tests/ItemViewReuseManagerTests.swift | 22 ++-- 5 files changed, 125 insertions(+), 109 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a58dc9..a58d8c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Added support for disabling touch handling on SwiftUI views via the `allowsHitTesting` modifier +### Fixed +- Fixed an issue that could cause accessibility focus to shift unexpectedly + +### Changed +- Rewrote accessibility code to avoid posting notifications, which causes poor Voice Over performance and odd focus bugs + ## [v2.0.0](https://github.com/airbnb/HorizonCalendar/compare/v1.16.0...v2.0.0) - 2023-12-19 ### Added diff --git a/Sources/Internal/ItemViewReuseManager.swift b/Sources/Internal/ItemViewReuseManager.swift index 4996989..e471039 100644 --- a/Sources/Internal/ItemViewReuseManager.swift +++ b/Sources/Internal/ItemViewReuseManager.swift @@ -25,7 +25,7 @@ final class ItemViewReuseManager { func viewsForVisibleItems( _ visibleItems: Set, - recycleOldViews: Bool, + recycleUnusedViews: Bool, viewHandler: ( ItemView, VisibleItem, @@ -57,7 +57,7 @@ final class ItemViewReuseManager { let context = reusedViewContext( for: visibleItem, - recycleOldViews: recycleOldViews, + recycleUnusedViews: recycleUnusedViews, unusedPreviouslyVisibleItems: &visibleItemsDifference) viewHandler( context.view, @@ -78,7 +78,7 @@ final class ItemViewReuseManager { private func reusedViewContext( for visibleItem: VisibleItem, - recycleOldViews: Bool, + recycleUnusedViews: Bool, unusedPreviouslyVisibleItems: inout Set) -> ReusedViewContext { @@ -106,7 +106,7 @@ final class ItemViewReuseManager { visibleItemsForItemViewDifferentiators[differentiator]?.remove(visibleItem) viewsForVisibleItems.removeValue(forKey: visibleItem) } else { - if recycleOldViews, let previouslyVisibleItem = unusedPreviouslyVisibleItems.first { + if recycleUnusedViews, let previouslyVisibleItem = unusedPreviouslyVisibleItems.first { // An unused, previously-visible item is available, so reuse it. guard let previousView = viewsForVisibleItems[previouslyVisibleItem] else { diff --git a/Sources/Internal/SubviewInsertionIndexTracker.swift b/Sources/Internal/SubviewInsertionIndexTracker.swift index 2abcb11..1f5c6b6 100644 --- a/Sources/Internal/SubviewInsertionIndexTracker.swift +++ b/Sources/Internal/SubviewInsertionIndexTracker.swift @@ -27,79 +27,33 @@ final class SubviewInsertionIndexTracker { switch itemType { case .monthBackground: index = monthBackgroundItemsEndIndex - monthBackgroundItemsEndIndex += 1 - dayRangeItemsEndIndex += 1 - mainItemsEndIndex += 1 - daysOfWeekRowSeparatorItemsEndIndex += 1 - overlayItemsEndIndex += 1 - pinnedDaysOfWeekRowBackgroundEndIndex += 1 - pinnedDayOfWeekItemsEndIndex += 1 - pinnedDaysOfWeekRowSeparatorEndIndex += 1 - case .dayBackground: index = dayBackgroundItemsEndIndex - dayBackgroundItemsEndIndex += 1 - dayRangeItemsEndIndex += 1 - mainItemsEndIndex += 1 - daysOfWeekRowSeparatorItemsEndIndex += 1 - overlayItemsEndIndex += 1 - pinnedDaysOfWeekRowBackgroundEndIndex += 1 - pinnedDayOfWeekItemsEndIndex += 1 - pinnedDaysOfWeekRowSeparatorEndIndex += 1 - case .dayRange: index = dayRangeItemsEndIndex - dayRangeItemsEndIndex += 1 - mainItemsEndIndex += 1 - daysOfWeekRowSeparatorItemsEndIndex += 1 - overlayItemsEndIndex += 1 - pinnedDaysOfWeekRowBackgroundEndIndex += 1 - pinnedDayOfWeekItemsEndIndex += 1 - pinnedDaysOfWeekRowSeparatorEndIndex += 1 - case .layoutItemType: index = mainItemsEndIndex - mainItemsEndIndex += 1 - daysOfWeekRowSeparatorItemsEndIndex += 1 - overlayItemsEndIndex += 1 - pinnedDaysOfWeekRowBackgroundEndIndex += 1 - pinnedDayOfWeekItemsEndIndex += 1 - pinnedDaysOfWeekRowSeparatorEndIndex += 1 - case .daysOfWeekRowSeparator: index = daysOfWeekRowSeparatorItemsEndIndex - daysOfWeekRowSeparatorItemsEndIndex += 1 - overlayItemsEndIndex += 1 - pinnedDaysOfWeekRowBackgroundEndIndex += 1 - pinnedDayOfWeekItemsEndIndex += 1 - pinnedDaysOfWeekRowSeparatorEndIndex += 1 - case .overlayItem: index = overlayItemsEndIndex - overlayItemsEndIndex += 1 - pinnedDaysOfWeekRowBackgroundEndIndex += 1 - pinnedDayOfWeekItemsEndIndex += 1 - pinnedDaysOfWeekRowSeparatorEndIndex += 1 - case .pinnedDaysOfWeekRowBackground: index = pinnedDaysOfWeekRowBackgroundEndIndex - pinnedDaysOfWeekRowBackgroundEndIndex += 1 - pinnedDayOfWeekItemsEndIndex += 1 - pinnedDaysOfWeekRowSeparatorEndIndex += 1 - case .pinnedDayOfWeek: index = pinnedDayOfWeekItemsEndIndex - pinnedDayOfWeekItemsEndIndex += 1 - pinnedDaysOfWeekRowSeparatorEndIndex += 1 - case .pinnedDaysOfWeekRowSeparator: index = pinnedDaysOfWeekRowSeparatorEndIndex - pinnedDaysOfWeekRowSeparatorEndIndex += 1 } + addValue(1, toItemTypesAffectedBy: itemType) + return index } + func removedSubview(withCorrespondingItemType itemType: VisibleItem.ItemType) { + addValue(-1, toItemTypesAffectedBy: itemType) + } + // MARK: Private private var monthBackgroundItemsEndIndex = 0 @@ -112,4 +66,70 @@ final class SubviewInsertionIndexTracker { private var pinnedDayOfWeekItemsEndIndex = 0 private var pinnedDaysOfWeekRowSeparatorEndIndex = 0 + private func addValue(_ value: Int, toItemTypesAffectedBy itemType: VisibleItem.ItemType) { + switch itemType { + case .monthBackground: + monthBackgroundItemsEndIndex += value + dayRangeItemsEndIndex += value + mainItemsEndIndex += value + daysOfWeekRowSeparatorItemsEndIndex += value + overlayItemsEndIndex += value + pinnedDaysOfWeekRowBackgroundEndIndex += value + pinnedDayOfWeekItemsEndIndex += value + pinnedDaysOfWeekRowSeparatorEndIndex += value + + case .dayBackground: + dayBackgroundItemsEndIndex += value + dayRangeItemsEndIndex += value + mainItemsEndIndex += value + daysOfWeekRowSeparatorItemsEndIndex += value + overlayItemsEndIndex += value + pinnedDaysOfWeekRowBackgroundEndIndex += value + pinnedDayOfWeekItemsEndIndex += value + pinnedDaysOfWeekRowSeparatorEndIndex += value + + case .dayRange: + dayRangeItemsEndIndex += value + mainItemsEndIndex += value + daysOfWeekRowSeparatorItemsEndIndex += value + overlayItemsEndIndex += value + pinnedDaysOfWeekRowBackgroundEndIndex += value + pinnedDayOfWeekItemsEndIndex += value + pinnedDaysOfWeekRowSeparatorEndIndex += value + + case .layoutItemType: + mainItemsEndIndex += value + daysOfWeekRowSeparatorItemsEndIndex += value + overlayItemsEndIndex += value + pinnedDaysOfWeekRowBackgroundEndIndex += value + pinnedDayOfWeekItemsEndIndex += value + pinnedDaysOfWeekRowSeparatorEndIndex += value + + case .daysOfWeekRowSeparator: + daysOfWeekRowSeparatorItemsEndIndex += value + overlayItemsEndIndex += value + pinnedDaysOfWeekRowBackgroundEndIndex += value + pinnedDayOfWeekItemsEndIndex += value + pinnedDaysOfWeekRowSeparatorEndIndex += value + + case .overlayItem: + overlayItemsEndIndex += value + pinnedDaysOfWeekRowBackgroundEndIndex += value + pinnedDayOfWeekItemsEndIndex += value + pinnedDaysOfWeekRowSeparatorEndIndex += value + + case .pinnedDaysOfWeekRowBackground: + pinnedDaysOfWeekRowBackgroundEndIndex += value + pinnedDayOfWeekItemsEndIndex += value + pinnedDaysOfWeekRowSeparatorEndIndex += value + + case .pinnedDayOfWeek: + pinnedDayOfWeekItemsEndIndex += value + pinnedDaysOfWeekRowSeparatorEndIndex += value + + case .pinnedDaysOfWeekRowSeparator: + pinnedDaysOfWeekRowSeparatorEndIndex += value + } + } + } diff --git a/Sources/Public/CalendarView.swift b/Sources/Public/CalendarView.swift index 591f1ec..486b1c6 100644 --- a/Sources/Public/CalendarView.swift +++ b/Sources/Public/CalendarView.swift @@ -180,8 +180,8 @@ public final class CalendarView: UIView { // Voice Over user experiencing "No heading found" when navigating by heading. We also check to // make sure an accessibility element has already been focused, otherwise the first // accessibility element will be off-screen when a user first focuses into the calendar view. - let extendLayoutRegion = UIAccessibility.isVoiceOverRunning && - itemTypeOfFocusedAccessibilityElement != nil + let extendLayoutRegion = UIAccessibility.isVoiceOverRunning && initialItemViewWasFocused + _layoutSubviews(extendLayoutRegion: extendLayoutRegion) } @@ -281,12 +281,6 @@ public final class CalendarView: UIView { UIView.animate(withDuration: 0.3, animations: animations) } } - - if UIAccessibility.isVoiceOverRunning { - DispatchQueue.main.async { - self.restoreAccessibilityFocusIfNeeded() - } - } } /// Returns the accessibility element associated with the specified visible date. If the date is not currently visible, then there will be no @@ -526,6 +520,14 @@ public final class CalendarView: UIView { private lazy var scrollViewDelegate = ScrollViewDelegate(calendarView: self) private lazy var gestureRecognizerDelegate = GestureRecognizerDelegate(calendarView: self) + private var initialItemViewWasFocused = false { + didSet { + guard initialItemViewWasFocused != oldValue else { return } + setNeedsLayout() + layoutIfNeeded() + } + } + // Necessary to work around a `UIScrollView` behavior difference on Mac. See `scrollViewDidScroll` // and `preventLargeOverScrollIfNeeded` for more context. private lazy var isRunningOnMac: Bool = { @@ -538,18 +540,6 @@ public final class CalendarView: UIView { return false }() - private var itemTypeOfFocusedAccessibilityElement: VisibleItem.ItemType? { - didSet { - switch (oldValue, itemTypeOfFocusedAccessibilityElement) { - case (.none, .some), (.some, .none): - setNeedsLayout() - layoutIfNeeded() - default: - break - } - } - } - private var isReadyForLayout: Bool { // There's no reason to attempt layout unless we have a non-zero `bounds.size`. We'll have a // non-zero size once the `frame` is set to something non-zero, either manually or via the @@ -785,6 +775,7 @@ public final class CalendarView: UIView { reuseManager.viewsForVisibleItems( visibleItems, + recycleUnusedViews: !UIAccessibility.isVoiceOverRunning, viewHandler: { view, visibleItem, previousBackingVisibleItem, isReusedViewSameAsPreviousView in UIView.conditionallyPerformWithoutAnimation(when: !isReusedViewSameAsPreviousView) { if view.superview == nil { @@ -807,8 +798,14 @@ public final class CalendarView: UIView { }) // Hide any old views that weren't reused. This is faster than adding / removing subviews. - for (_, viewToHide) in viewsToHideForVisibleItems { - viewToHide.isHidden = true + // If VoiceOver is running, we remove the view to save memory (since views aren't reused). + for (visibleItem, viewToHide) in viewsToHideForVisibleItems { + if UIAccessibility.isVoiceOverRunning { + viewToHide.removeFromSuperview() + subviewInsertionIndexTracker.removedSubview(withCorrespondingItemType: visibleItem.itemType) + } else { + viewToHide.isHidden = true + } } } @@ -1222,31 +1219,20 @@ extension CalendarView { // MARK: Private - private func restoreAccessibilityFocusIfNeeded() { - let itemView = visibleViewsForVisibleItems.values.first { - $0.itemType == itemTypeOfFocusedAccessibilityElement - } - guard let itemView else { return } - - // Preserve the focused accessibility element after views are reused due to a content update - UIAccessibility.post(notification: .screenChanged, argument: itemView.contentView) - } - @objc private func accessibilityElementFocused(_ notification: NSNotification) { guard let element = notification.userInfo?[UIAccessibility.focusedElementUserInfoKey] as? UIResponder, let itemView = element.nextItemView() else { - itemTypeOfFocusedAccessibilityElement = nil return } - itemTypeOfFocusedAccessibilityElement = itemView.itemType + initialItemViewWasFocused = true - // If the accessibility element is not fully in view programmatically scroll it to be centered. + // If the accessibility element is not fully in view, programmatically scroll it to be centered. let isElementFullyVisible: Bool - let viewFrameInCalendarView = convert(itemView.bounds, from: itemView) + let viewFrameInCalendarView = itemView.convert(itemView.bounds, to: self) switch scrollMetricsMutator.scrollAxis { case .vertical: let verticalBounds = CGRect( @@ -1254,22 +1240,26 @@ extension CalendarView { y: layoutMargins.top, width: bounds.width, height: bounds.height - layoutMargins.top - layoutMargins.bottom) - isElementFullyVisible = !verticalBounds.contains(viewFrameInCalendarView) + isElementFullyVisible = verticalBounds.contains(viewFrameInCalendarView) case .horizontal: let horizontalBounds = CGRect( x: layoutMargins.left, y: 0, width: bounds.width - layoutMargins.left - layoutMargins.right, height: bounds.height) - isElementFullyVisible = !horizontalBounds.contains(viewFrameInCalendarView) + isElementFullyVisible = horizontalBounds.contains(viewFrameInCalendarView) } - if isElementFullyVisible { - switch itemView.itemType { - case .layoutItemType(.monthHeader(let month)): + if + !isElementFullyVisible, + let itemType = itemView.itemType, + case .layoutItemType(let layoutItemType) = itemType + { + switch layoutItemType { + case .monthHeader(let month): let dateInTargetMonth = calendar.firstDate(of: month) scroll(toMonthContaining: dateInTargetMonth, scrollPosition: .centered, animated: false) - case .layoutItemType(.day(let day)): + case .day(let day): let dateInTargetDay = calendar.startDate(of: day) scroll(toDayContaining: dateInTargetDay, scrollPosition: .centered, animated: false) default: diff --git a/Tests/ItemViewReuseManagerTests.swift b/Tests/ItemViewReuseManagerTests.swift index d95aeb9..bef1c74 100644 --- a/Tests/ItemViewReuseManagerTests.swift +++ b/Tests/ItemViewReuseManagerTests.swift @@ -58,7 +58,7 @@ final class ItemViewReuseManagerTests: XCTestCase { reuseManager.viewsForVisibleItems( visibleItems, - recycleOldViews: true, + recycleUnusedViews: true, viewHandler: { _, _, previousBackingItem, isReusedViewSameAsPreviousView in XCTAssert( previousBackingItem == nil, @@ -104,13 +104,13 @@ final class ItemViewReuseManagerTests: XCTestCase { // Populate the reuse manager with the initial visible items reuseManager.viewsForVisibleItems( initialVisibleItems, - recycleOldViews: true, + recycleUnusedViews: true, viewHandler: { _, _, _, _ in }) // Ensure all views are reused by using the exact same previous views reuseManager.viewsForVisibleItems( subsequentVisibleItems, - recycleOldViews: true, + recycleUnusedViews: true, viewHandler: { _, item, previousBackingItem, isReusedViewSameAsPreviousView in XCTAssert( item == previousBackingItem, @@ -186,13 +186,13 @@ final class ItemViewReuseManagerTests: XCTestCase { // Populate the reuse manager with the initial visible items reuseManager.viewsForVisibleItems( initialVisibleItems, - recycleOldViews: true, + recycleUnusedViews: true, viewHandler: { _, _, _, _ in }) // Ensure all views are reused given the subsequent visible items reuseManager.viewsForVisibleItems( subsequentVisibleItems, - recycleOldViews: true, + recycleUnusedViews: true, viewHandler: { _, item, previousBackingItem, _ in XCTAssert( item.calendarItemModel._itemViewDifferentiator == previousBackingItem?.calendarItemModel._itemViewDifferentiator, @@ -281,13 +281,13 @@ final class ItemViewReuseManagerTests: XCTestCase { // Populate the reuse manager with the initial visible items reuseManager.viewsForVisibleItems( initialVisibleItems, - recycleOldViews: true, + recycleUnusedViews: true, viewHandler: { _, _, _, _ in }) // Ensure the correct subset of views are reused given the subsequent visible items reuseManager.viewsForVisibleItems( subsequentVisibleItems, - recycleOldViews: true, + recycleUnusedViews: true, viewHandler: { _, item, previousBackingItem, isReusedViewSameAsPreviousView in guard let itemModel = item.calendarItemModel as? MockCalendarItemModel else { preconditionFailure( @@ -423,7 +423,7 @@ final class ItemViewReuseManagerTests: XCTestCase { // Populate the reuse manager with the initial visible items reuseManager.viewsForVisibleItems( initialVisibleItems, - recycleOldViews: true, + recycleUnusedViews: true, viewHandler: { _, _, _, _ in }) // Ensure the correct subset of views are reused given the subsequent visible items @@ -431,7 +431,7 @@ final class ItemViewReuseManagerTests: XCTestCase { var newViewCountsForDifferentiators = [_CalendarItemViewDifferentiator: Int]() reuseManager.viewsForVisibleItems( subsequentVisibleItems, - recycleOldViews: true, + recycleUnusedViews: true, viewHandler: { _, item, previousBackingItem, _ in if previousBackingItem != nil { let reuseCount = (reuseCountsForDifferentiators[item.calendarItemModel._itemViewDifferentiator] ?? 0) + 1 @@ -539,13 +539,13 @@ final class ItemViewReuseManagerTests: XCTestCase { // Populate the reuse manager with the initial visible items reuseManager.viewsForVisibleItems( initialVisibleItems, - recycleOldViews: false, + recycleUnusedViews: false, viewHandler: { _, _, _, _ in }) // Ensure the correct subset of views are reused given the subsequent visible items reuseManager.viewsForVisibleItems( subsequentVisibleItems, - recycleOldViews: false, + recycleUnusedViews: false, viewHandler: { _, item, previousBackingItem, isReusedViewSameAsPreviousView in guard let itemModel = item.calendarItemModel as? MockCalendarItemModel else { preconditionFailure( From 1c5e9acc8a1b16125092a3642a8cd65b87489a21 Mon Sep 17 00:00:00 2001 From: Bryan Keller Date: Thu, 25 Jan 2024 23:04:23 -0800 Subject: [PATCH 3/3] Update Github actions --- .github/workflows/swift.yml | 21 ++++++++++++--------- HorizonCalendar.podspec | 4 ++-- HorizonCalendar.xcodeproj/project.pbxproj | 10 ++++++---- Package.swift | 5 ++--- Sources/Public/CalendarView.swift | 16 ++++++++-------- 5 files changed, 30 insertions(+), 26 deletions(-) diff --git a/.github/workflows/swift.yml b/.github/workflows/swift.yml index f997f83..60dfb61 100644 --- a/.github/workflows/swift.yml +++ b/.github/workflows/swift.yml @@ -1,4 +1,4 @@ -name: Swift +name: CI on: push: @@ -8,22 +8,25 @@ on: jobs: build: - runs-on: macos-latest + runs-on: macos-13 strategy: matrix: - destination: ['platform=iOS Simulator,OS=11.0,name=iPhone 8', 'platform=iOS Simulator,OS=16.2,name=iPhone 14'] + xcode: + - '15.0' # Swift 5.9 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Build - run: xcodebuild clean build -scheme HorizonCalendar + run: xcodebuild clean build -scheme HorizonCalendar -destination "generic/platform=iOS Simulator" - name: Run tests - run: xcodebuild clean test -project HorizonCalendar.xcodeproj -scheme HorizonCalendar -destination "name=iPhone 8,OS=16.2" + run: xcodebuild clean test -project HorizonCalendar.xcodeproj -scheme HorizonCalendar -destination "name=iPhone 14,OS=17.2" lint-swift: runs-on: macos-13 + strategy: + matrix: + xcode: + - '15.0' # Swift 5.9 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Lint Swift run: swift package --allow-writing-to-package-directory format --lint - - diff --git a/HorizonCalendar.podspec b/HorizonCalendar.podspec index 6a959c1..5c2942c 100644 --- a/HorizonCalendar.podspec +++ b/HorizonCalendar.podspec @@ -36,7 +36,7 @@ Features: spec.homepage = "https://github.com/airbnb/HorizonCalendar" spec.authors = { "Bryan Keller" => "kellerbryan19@gmail.com" } spec.social_media_url = "https://twitter.com/BKYourWay19" - spec.swift_version = "5.8" - spec.ios.deployment_target = '11.0' + spec.swift_version = "5.9" + spec.ios.deployment_target = '12.0' spec.source_files = "Sources/**/*.{swift,h}" end diff --git a/HorizonCalendar.xcodeproj/project.pbxproj b/HorizonCalendar.xcodeproj/project.pbxproj index 4d870e6..59b19f7 100644 --- a/HorizonCalendar.xcodeproj/project.pbxproj +++ b/HorizonCalendar.xcodeproj/project.pbxproj @@ -565,6 +565,7 @@ SDKROOT = iphoneos; SWIFT_ACTIVE_COMPILATION_CONDITIONS = DEBUG; SWIFT_OPTIMIZATION_LEVEL = "-Onone"; + SWIFT_VERSION = 5.0; VERSIONING_SYSTEM = "apple-generic"; VERSION_INFO_PREFIX = ""; }; @@ -622,6 +623,7 @@ SDKROOT = iphoneos; SWIFT_COMPILATION_MODE = wholemodule; SWIFT_OPTIMIZATION_LEVEL = "-O"; + SWIFT_VERSION = 5.0; VALIDATE_PRODUCT = YES; VERSIONING_SYSTEM = "apple-generic"; VERSION_INFO_PREFIX = ""; @@ -644,7 +646,7 @@ GCC_WARN_UNUSED_PARAMETER = YES; INFOPLIST_FILE = Info.plist; INSTALL_PATH = "$(LOCAL_LIBRARY_DIR)/Frameworks"; - IPHONEOS_DEPLOYMENT_TARGET = 11.0; + IPHONEOS_DEPLOYMENT_TARGET = 12.0; LD_RUNPATH_SEARCH_PATHS = ( "$(inherited)", "@executable_path/Frameworks", @@ -678,7 +680,7 @@ GCC_WARN_UNUSED_PARAMETER = YES; INFOPLIST_FILE = Info.plist; INSTALL_PATH = "$(LOCAL_LIBRARY_DIR)/Frameworks"; - IPHONEOS_DEPLOYMENT_TARGET = 11.0; + IPHONEOS_DEPLOYMENT_TARGET = 12.0; LD_RUNPATH_SEARCH_PATHS = ( "$(inherited)", "@executable_path/Frameworks", @@ -702,7 +704,7 @@ CODE_SIGN_STYLE = Automatic; DEVELOPMENT_TEAM = 5Q5SGQT2R4; INFOPLIST_FILE = Tests/Info.plist; - IPHONEOS_DEPLOYMENT_TARGET = 11.0; + IPHONEOS_DEPLOYMENT_TARGET = 12.0; LD_RUNPATH_SEARCH_PATHS = ( "$(inherited)", "@executable_path/Frameworks", @@ -722,7 +724,7 @@ CODE_SIGN_STYLE = Automatic; DEVELOPMENT_TEAM = 5Q5SGQT2R4; INFOPLIST_FILE = Tests/Info.plist; - IPHONEOS_DEPLOYMENT_TARGET = 11.0; + IPHONEOS_DEPLOYMENT_TARGET = 12.0; LD_RUNPATH_SEARCH_PATHS = ( "$(inherited)", "@executable_path/Frameworks", diff --git a/Package.swift b/Package.swift index ab6551f..236e7fa 100644 --- a/Package.swift +++ b/Package.swift @@ -1,11 +1,10 @@ -// swift-tools-version:5.8 - +// swift-tools-version: 5.8.1 import PackageDescription let package = Package( name: "HorizonCalendar", platforms: [ - .iOS(.v11), + .iOS(.v12), ], products: [ .library(name: "HorizonCalendar", targets: ["HorizonCalendar"]), diff --git a/Sources/Public/CalendarView.swift b/Sources/Public/CalendarView.swift index 486b1c6..fdaf821 100644 --- a/Sources/Public/CalendarView.swift +++ b/Sources/Public/CalendarView.swift @@ -520,14 +520,6 @@ public final class CalendarView: UIView { private lazy var scrollViewDelegate = ScrollViewDelegate(calendarView: self) private lazy var gestureRecognizerDelegate = GestureRecognizerDelegate(calendarView: self) - private var initialItemViewWasFocused = false { - didSet { - guard initialItemViewWasFocused != oldValue else { return } - setNeedsLayout() - layoutIfNeeded() - } - } - // Necessary to work around a `UIScrollView` behavior difference on Mac. See `scrollViewDidScroll` // and `preventLargeOverScrollIfNeeded` for more context. private lazy var isRunningOnMac: Bool = { @@ -540,6 +532,14 @@ public final class CalendarView: UIView { return false }() + private var initialItemViewWasFocused = false { + didSet { + guard initialItemViewWasFocused != oldValue else { return } + setNeedsLayout() + layoutIfNeeded() + } + } + private var isReadyForLayout: Bool { // There's no reason to attempt layout unless we have a non-zero `bounds.size`. We'll have a // non-zero size once the `frame` is set to something non-zero, either manually or via the