Skip to content
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

Fix #184: Incorrect tap area for Right-to-Left languages #194

Merged
merged 11 commits into from
Mar 20, 2024
Merged
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ Guide: https://keepachangelog.com/en/1.0.0/

<!-- Add changes for active work here -->

- [UI] Add Right-to-Left language support for Categories/Favorites segment control and fix xib errors.
- [UI] Add Preview file for CategoriesFavoritesSegmentControl to fix compiler problems.

- [Core] Add SearchError.owningObjectDeallocated when network responses fail to unwrap guard-let-self. If you encounter this error you must own the reference to the search engine.
- [Tests] Add UnownedObjectError tests to validate behavior of SearchError.owningObjectDeallocated.

Expand Down
4 changes: 4 additions & 0 deletions MapboxSearch.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
042477C62B72CCB000D870D5 /* geocoding-reverse-geocoding.json in Resources */ = {isa = PBXBuildFile; fileRef = 042477C42B72CCB000D870D5 /* geocoding-reverse-geocoding.json */; };
042477C72B72CCB000D870D5 /* geocoding-reverse-geocoding.json in Resources */ = {isa = PBXBuildFile; fileRef = 042477C42B72CCB000D870D5 /* geocoding-reverse-geocoding.json */; };
043A3D4D2B30F38300DB681B /* CoreAddress+AddressComponents.swift in Sources */ = {isa = PBXBuildFile; fileRef = 043A3D4C2B30F38300DB681B /* CoreAddress+AddressComponents.swift */; };
044A6B732BA8933200A9F2A2 /* PreviewCategoriesFavoritesSegmentControl.swift in Sources */ = {isa = PBXBuildFile; fileRef = 044A6B722BA8933200A9F2A2 /* PreviewCategoriesFavoritesSegmentControl.swift */; };
045514C22B7D4B58000D88B9 /* CoreApiType+ToSDKType.swift in Sources */ = {isa = PBXBuildFile; fileRef = 045514C12B7D4B58000D88B9 /* CoreApiType+ToSDKType.swift */; };
045514C32B7D4B58000D88B9 /* CoreApiType+ToSDKType.swift in Sources */ = {isa = PBXBuildFile; fileRef = 045514C12B7D4B58000D88B9 /* CoreApiType+ToSDKType.swift */; };
045514C42B7D4B58000D88B9 /* CoreApiType+ToSDKType.swift in Sources */ = {isa = PBXBuildFile; fileRef = 045514C12B7D4B58000D88B9 /* CoreApiType+ToSDKType.swift */; };
Expand Down Expand Up @@ -533,6 +534,7 @@
042477C12B7290E700D870D5 /* SearchEngineGeocodingIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SearchEngineGeocodingIntegrationTests.swift; sourceTree = "<group>"; };
042477C42B72CCB000D870D5 /* geocoding-reverse-geocoding.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "geocoding-reverse-geocoding.json"; sourceTree = "<group>"; };
043A3D4C2B30F38300DB681B /* CoreAddress+AddressComponents.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CoreAddress+AddressComponents.swift"; sourceTree = "<group>"; };
044A6B722BA8933200A9F2A2 /* PreviewCategoriesFavoritesSegmentControl.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PreviewCategoriesFavoritesSegmentControl.swift; sourceTree = "<group>"; };
045514C12B7D4B58000D88B9 /* CoreApiType+ToSDKType.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CoreApiType+ToSDKType.swift"; sourceTree = "<group>"; };
046818D32B87F2A70082B188 /* SearchBox_CategorySearchEngineIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SearchBox_CategorySearchEngineIntegrationTests.swift; sourceTree = "<group>"; };
046818DA2B87FAB20082B188 /* search-box-category.json */ = {isa = PBXFileReference; explicitFileType = text.json; path = "search-box-category.json"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1835,6 +1837,7 @@
FEEDD3472508E02700DC0A98 /* AddToFavoritesCell.xib */,
FEEDD3352508E02700DC0A98 /* Assets.xcassets */,
FEEDD3412508E02700DC0A98 /* CategoriesFavoritesSegmentControl.swift */,
044A6B722BA8933200A9F2A2 /* PreviewCategoriesFavoritesSegmentControl.swift */,
FEEDD34C2508E02700DC0A98 /* CategoriesFavoritesSegmentControl.xib */,
FEEDD34B2508E02700DC0A98 /* CategoriesProvider.swift */,
FEEDD32E2508E02700DC0A98 /* CategoriesTableViewCell.swift */,
Expand Down Expand Up @@ -2893,6 +2896,7 @@
FE49CF6E2510EFD00059C189 /* DefaultCategoryDataProvider.swift in Sources */,
F9C98874270B42FA00D030B0 /* LocationCoordinateHelpers.swift in Sources */,
FEEDD3782508E02700DC0A98 /* FavoriteEntry.swift in Sources */,
044A6B732BA8933200A9F2A2 /* PreviewCategoriesFavoritesSegmentControl.swift in Sources */,
FEEDD3562508E02700DC0A98 /* MapboxSearchController.swift in Sources */,
FEEDD3682508E02700DC0A98 /* HistoryHeader.swift in Sources */,
FEEDD37F2508E02700DC0A98 /* ActivityProgressView.swift in Sources */,
Expand Down
33 changes: 1 addition & 32 deletions Sources/MapboxSearchUI/CategoriesFavoritesSegmentControl.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import UIKit

// Preview available in PreviewCategoriesFavoritesSegmentControl.swift
class CategoriesFavoritesSegmentControl: UIControl {
enum Tab {
case categories
Expand Down Expand Up @@ -40,8 +41,6 @@ class CategoriesFavoritesSegmentControl: UIControl {
@IBOutlet private var favoritesInactiveTitle: UIButton!

@IBOutlet private var selectionSegment: UIView!
@IBOutlet private var selectionCategoriesHorizontalConstraint: NSLayoutConstraint!
@IBOutlet private var selectionFavoritesHorizontalConstraint: NSLayoutConstraint!

var configuration: Configuration! {
didSet {
Expand Down Expand Up @@ -163,33 +162,3 @@ class CategoriesFavoritesSegmentControl: UIControl {
favoritesInactiveTitleMask.path = selectionSegmentPath
}
}

#if canImport(SwiftUI) && DEBUG
import SwiftUI

@available(iOS 13.0, *)
struct TabsSegmentControlRepresentable: UIViewRepresentable {
func makeUIView(context: Context) -> UIView {
UINib(nibName: "CategoriesFavoritesSegmentControl", bundle: .mapboxSearchUI)
.instantiate(withOwner: nil, options: nil)[0] as! UIView
// swiftlint:disable:previous force_cast
}

func updateUIView(_ view: UIView, context: Context) {}
}

@available(iOS 13.0, *)
struct CategoriesFavoritesSegmentControlPreview: PreviewProvider {
static var previews: some View {
Group {
TabsSegmentControlRepresentable()
.previewDisplayName("Light Mode")
.previewLayout(PreviewLayout.fixed(width: 202, height: 28))
TabsSegmentControlRepresentable()
.previewDisplayName("Dark Mode")
.preferredColorScheme(.dark)
.previewLayout(PreviewLayout.fixed(width: 300, height: 40))
}
}
}
#endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright © 2024 Mapbox. All rights reserved.

import Foundation

#if canImport(SwiftUI) && DEBUG
import SwiftUI

@available(iOS 13.0, *)
struct TabsSegmentControlRepresentable: UIViewRepresentable {
func makeUIView(context: Context) -> UIView {
let segmentControl: CategoriesFavoritesSegmentControl = UINib(
nibName: "CategoriesFavoritesSegmentControl",
bundle: .mapboxSearchUI
)
.instantiate(withOwner: nil, options: nil)[0] as! CategoriesFavoritesSegmentControl
// swiftlint:disable:previous force_cast

segmentControl.configuration = Configuration()

return segmentControl
}

func updateUIView(_ view: UIView, context: Context) {}
}

@available(iOS 13.0, *)
struct CategoriesFavoritesSegmentControlPreview: PreviewProvider {
static var previews: some View {
Group {
TabsSegmentControlRepresentable()
.previewDisplayName("Light Mode")
.previewLayout(PreviewLayout.fixed(width: 202, height: 28))
TabsSegmentControlRepresentable()
.previewDisplayName("Dark Mode")
.preferredColorScheme(.dark)
.previewLayout(PreviewLayout.fixed(width: 202, height: 28))
.previewLayout(.sizeThatFits)
TabsSegmentControlRepresentable()
.previewDisplayName("Right to Left")
.previewLayout(PreviewLayout.fixed(width: 202, height: 28))
.environment(\.layoutDirection, .rightToLeft)
}
}
}
#endif
75 changes: 66 additions & 9 deletions Sources/MapboxSearchUI/SearchCategoriesRootView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,42 @@ class SearchCategoriesRootView: UIView {
categoriesTableView.separatorColor = configuration.style.separatorColor
}

override func layoutSubviews() {
super.layoutSubviews()

/// Make sure that RTL users display the default tab
/// The scroll view will start at content offset (0, 0)
/// but the start tab will register as favorites.
/// When starting out with RTL, the current tab is favorites and != to `.categories` tab,
/// then manually set the content offset to the appropriate `.categories` default tab.
if effectiveUserInterfaceLayoutDirection == .rightToLeft {
let page = contentScrollView.contentOffset.x / contentScrollView.bounds.width
let currentTab = CategoriesFavoritesSegmentControl.Tab(
scrollViewPageProgress: page,
direction: effectiveUserInterfaceLayoutDirection
)

let defaultTab = CategoriesFavoritesSegmentControl.Tab.categories
guard currentTab != defaultTab else {
return
}

contentScrollView.contentOffset.x = defaultTab.horizontalOffsetFor(scrollView: contentScrollView)

// Without forcing a refresh the titles and masks will not display correctly (invisible or grayed-out)
// Force another layout pass to ensure these display correctly.
segmentedControl.setNeedsLayout()
segmentedControl.setNeedsDisplay()
segmentedControl.layoutIfNeeded()

// On first-draw we have just assigned the tab to the default and we know
// that this will render incorrectly for RTL users.
// Re-assigning the progress to the (backwards) location of the second tab
// (really "first tab" (which is zero-indexed)) will force it to redraw correctly.
segmentedControl.selectionSegmentProgress = 1
}
}

func resetUI(animated: Bool) {
contentScrollView.setContentOffset(.zero, animated: animated)
}
Expand Down Expand Up @@ -139,7 +175,10 @@ extension SearchCategoriesRootView: FavoritesTableViewSourceDelegate {
extension SearchCategoriesRootView: UIScrollViewDelegate {
func scrollViewDidScroll(_ scrollView: UIScrollView) {
let page = scrollView.contentOffset.x / scrollView.bounds.width
let newTab = CategoriesFavoritesSegmentControl.Tab(scrollViewPageProgress: page)
let newTab = CategoriesFavoritesSegmentControl.Tab(
scrollViewPageProgress: page,
direction: scrollView.effectiveUserInterfaceLayoutDirection
)

if segmentedControl.selectedTab != newTab {
segmentedControl.selectedTab = newTab
Expand All @@ -159,7 +198,8 @@ extension SearchCategoriesRootView {
.beginFromCurrentState,
.allowUserInteraction,
.curveEaseInOut,
], animations: {
], animations: { [weak self] in
guard let self else { return }
self.contentScrollView.contentOffset.x = self.segmentedControl.selectedTab.horizontalOffsetFor(
scrollView:
self.contentScrollView
Expand All @@ -169,20 +209,37 @@ extension SearchCategoriesRootView {
}

extension CategoriesFavoritesSegmentControl.Tab {
fileprivate init(scrollViewPageProgress: CGFloat) {
if scrollViewPageProgress <= 0.5 {
self = .categories
/// When the scroll view is instantiated and at the default location, show the categories Tab
///
fileprivate init(scrollViewPageProgress: CGFloat, direction: UIUserInterfaceLayoutDirection) {
if direction == .leftToRight {
if scrollViewPageProgress <= 0.5 {
self = .categories
} else {
self = .favorites
}
} else {
self = .favorites
/// Right to Left behavior
if scrollViewPageProgress <= 0.5 {
self = .favorites
} else {
self = .categories
}
}
}

fileprivate func horizontalOffsetFor(scrollView: UIScrollView) -> CGFloat {
switch self {
case .categories:
switch (self, scrollView.effectiveUserInterfaceLayoutDirection) {
case (.categories, .leftToRight):
return 0
case .favorites:
case (.favorites, .leftToRight):
return scrollView.bounds.width
case (.categories, .rightToLeft):
return scrollView.bounds.width
case (.favorites, .rightToLeft):
return 0
case (_, _):
fatalError("Unsupported text direction")
}
}
}
17 changes: 10 additions & 7 deletions Sources/MapboxSearchUI/SearchCategoriesRootView.xib
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
<?xml version="1.0" encoding="UTF-8"?>
<document type="com.apple.InterfaceBuilder3.CocoaTouch.XIB" version="3.0" toolsVersion="17701" targetRuntime="iOS.CocoaTouch" propertyAccessControl="none" useAutolayout="YES" useTraitCollections="YES" useSafeAreas="YES" colorMatched="YES">
<document type="com.apple.InterfaceBuilder3.CocoaTouch.XIB" version="3.0" toolsVersion="21507" targetRuntime="iOS.CocoaTouch" propertyAccessControl="none" useAutolayout="YES" useTraitCollections="YES" useSafeAreas="YES" colorMatched="YES">
<device id="retina5_9" orientation="portrait" appearance="dark"/>
<dependencies>
<deployment identifier="iOS"/>
<plugIn identifier="com.apple.InterfaceBuilder.IBCocoaTouchPlugin" version="17703"/>
<plugIn identifier="com.apple.InterfaceBuilder.IBCocoaTouchPlugin" version="21505"/>
<capability name="Named colors" minToolsVersion="9.0"/>
<capability name="Safe area layout guides" minToolsVersion="9.0"/>
<capability name="System colors in document resources" minToolsVersion="11.0"/>
Expand Down Expand Up @@ -43,19 +43,19 @@
<constraint firstItem="Feo-4t-hL3" firstAttribute="height" secondItem="EeJ-Pr-68t" secondAttribute="height" id="mcZ-9I-5hW"/>
</constraints>
</stackView>
<scrollView clipsSubviews="YES" multipleTouchEnabled="YES" contentMode="scaleToFill" ambiguous="YES" preservesSuperviewLayoutMargins="YES" pagingEnabled="YES" showsHorizontalScrollIndicator="NO" showsVerticalScrollIndicator="NO" translatesAutoresizingMaskIntoConstraints="NO" id="h9D-J2-ZgY">
<scrollView clipsSubviews="YES" multipleTouchEnabled="YES" contentMode="scaleToFill" preservesSuperviewLayoutMargins="YES" pagingEnabled="YES" showsHorizontalScrollIndicator="NO" showsVerticalScrollIndicator="NO" translatesAutoresizingMaskIntoConstraints="NO" id="h9D-J2-ZgY">
<rect key="frame" x="16" y="148.66666666666669" width="343" height="663.33333333333326"/>
<subviews>
<view contentMode="scaleToFill" ambiguous="YES" preservesSuperviewLayoutMargins="YES" translatesAutoresizingMaskIntoConstraints="NO" id="OuV-Ju-JWA">
<view contentMode="scaleToFill" preservesSuperviewLayoutMargins="YES" translatesAutoresizingMaskIntoConstraints="NO" id="OuV-Ju-JWA">
<rect key="frame" x="0.0" y="0.0" width="686" height="663.33333333333337"/>
<subviews>
<tableView clipsSubviews="YES" contentMode="scaleToFill" ambiguous="YES" preservesSuperviewLayoutMargins="YES" alwaysBounceVertical="YES" style="plain" separatorStyle="default" rowHeight="-1" estimatedRowHeight="-1" sectionHeaderHeight="28" sectionFooterHeight="28" translatesAutoresizingMaskIntoConstraints="NO" id="ce7-2P-7hl">
<tableView clipsSubviews="YES" contentMode="scaleToFill" preservesSuperviewLayoutMargins="YES" alwaysBounceVertical="YES" style="plain" separatorStyle="default" rowHeight="-1" estimatedRowHeight="-1" sectionHeaderHeight="28" sectionFooterHeight="28" translatesAutoresizingMaskIntoConstraints="NO" id="ce7-2P-7hl">
<rect key="frame" x="0.0" y="0.0" width="343" height="663.33333333333337"/>
<color key="backgroundColor" name="mapbox background"/>
<color key="separatorColor" name="mapbox separator"/>
<inset key="separatorInset" minX="0.0" minY="0.0" maxX="0.0" maxY="0.0"/>
</tableView>
<tableView clipsSubviews="YES" contentMode="scaleToFill" ambiguous="YES" alwaysBounceVertical="YES" style="plain" separatorStyle="default" rowHeight="-1" estimatedRowHeight="-1" sectionHeaderHeight="28" sectionFooterHeight="28" translatesAutoresizingMaskIntoConstraints="NO" id="7w2-Vh-qmn">
<tableView clipsSubviews="YES" contentMode="scaleToFill" alwaysBounceVertical="YES" style="plain" separatorStyle="default" rowHeight="-1" estimatedRowHeight="-1" sectionHeaderHeight="28" sectionFooterHeight="28" translatesAutoresizingMaskIntoConstraints="NO" id="7w2-Vh-qmn">
<rect key="frame" x="343" y="0.0" width="343" height="663.33333333333337"/>
<color key="backgroundColor" name="mapbox background"/>
<color key="separatorColor" name="mapbox separator"/>
Expand Down Expand Up @@ -90,9 +90,12 @@
<outlet property="delegate" destination="iN0-l3-epB" id="rwA-mI-D8Q"/>
</connections>
</scrollView>
<view contentMode="scaleToFill" ambiguous="YES" restorationIdentifier="CategoriesFavoritesSegmentControl" translatesAutoresizingMaskIntoConstraints="NO" id="hGT-OH-Edh" customClass="PlaceholderView" customModule="MapboxSearchUI">
<view contentMode="scaleToFill" restorationIdentifier="CategoriesFavoritesSegmentControl" translatesAutoresizingMaskIntoConstraints="NO" id="hGT-OH-Edh" customClass="PlaceholderView" customModule="MapboxSearchUI">
<rect key="frame" x="86.666666666666686" y="110.66666666666667" width="202" height="28.000000000000014"/>
<color key="backgroundColor" systemColor="systemBackgroundColor"/>
<constraints>
<constraint firstAttribute="height" constant="28" id="ag7-gv-xib"/>
</constraints>
</view>
</subviews>
<viewLayoutGuide key="safeArea" id="vUN-kp-3ea"/>
Expand Down