Simplify accessibility implementation in CalendarView

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

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
+    }
+  }
+
 }

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 && hasHadInitialItemViewFocus
+
     _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 hasHadInitialItemViewFocus = false {
+    didSet {
+      guard hasHadInitialItemViewFocus != 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,
+      recycleOldViews: !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,54 +1219,47 @@ 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
+    hasHadInitialItemViewFocus = 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(
         x: 0,
         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: