diff --git a/iphone/Chart/Chart/Views/ChartLineView.swift b/iphone/Chart/Chart/Views/ChartLineView.swift index 872271784..9daab8626 100644 --- a/iphone/Chart/Chart/Views/ChartLineView.swift +++ b/iphone/Chart/Chart/Views/ChartLineView.swift @@ -67,7 +67,7 @@ class ChartLineView: UIView { } func setY(min: CGFloat, max: CGFloat, animationStyle: ChartAnimation = .none) { - assert(min < max) + assert(min <= max) minY = min maxY = max updateGraph(animationStyle: animationStyle) diff --git a/iphone/CoreApi/CoreApi/PlacePageData/Common/PlacePagePreviewData.mm b/iphone/CoreApi/CoreApi/PlacePageData/Common/PlacePagePreviewData.mm index 199c1b5e0..eaa1527b3 100644 --- a/iphone/CoreApi/CoreApi/PlacePageData/Common/PlacePagePreviewData.mm +++ b/iphone/CoreApi/CoreApi/PlacePageData/Common/PlacePagePreviewData.mm @@ -53,7 +53,6 @@ static PlacePageDataSchedule convertOpeningHours(std::string_view rawOH) - (instancetype)initWithTrackInfo:(TrackInfo * _Nonnull)trackInfo { self = [super init]; if (self) { - // TODO: (KK) Replace separator with a shared static constant. NSString * kSeparator = @" • "; _title = [@[trackInfo.duration, trackInfo.distance] componentsJoinedByString:kSeparator]; } diff --git a/iphone/CoreApi/CoreApi/PlacePageData/ElevationProfile/ElevationProfileData.mm b/iphone/CoreApi/CoreApi/PlacePageData/ElevationProfile/ElevationProfileData.mm index ffdd63c2d..e09cfd38f 100644 --- a/iphone/CoreApi/CoreApi/PlacePageData/ElevationProfile/ElevationProfileData.mm +++ b/iphone/CoreApi/CoreApi/PlacePageData/ElevationProfile/ElevationProfileData.mm @@ -59,7 +59,7 @@ static ElevationDifficulty convertDifficulty(uint8_t difficulty) { andAltitude:point.m_point.GetAltitude()]; [pointsArray addObject:elevationPoint]; } - return [pointsArray copy]; + return pointsArray; } @end diff --git a/iphone/Maps/Classes/CustomViews/MapViewControls/TrackRecordingButtonViewController.swift b/iphone/Maps/Classes/CustomViews/MapViewControls/TrackRecordingButtonViewController.swift index fd864d2b0..33a07c247 100644 --- a/iphone/Maps/Classes/CustomViews/MapViewControls/TrackRecordingButtonViewController.swift +++ b/iphone/Maps/Classes/CustomViews/MapViewControls/TrackRecordingButtonViewController.swift @@ -41,10 +41,7 @@ final class TrackRecordingButtonViewController: MWMViewController { override func viewDidLoad() { super.viewDidLoad() - // async is for smoother appearance - DispatchQueue.main.asyncAfter(deadline: .now() + kDefaultAnimationDuration) { - self.setState(self.state, completion: nil) - } + setState(self.state, completion: nil) } // MARK: - Public methods diff --git a/iphone/Maps/Classes/MapViewController.mm b/iphone/Maps/Classes/MapViewController.mm index 606c22f0d..4ad1ec01d 100644 --- a/iphone/Maps/Classes/MapViewController.mm +++ b/iphone/Maps/Classes/MapViewController.mm @@ -892,7 +892,7 @@ NSString *const kSettingsSegue = @"Map2Settings"; TrackInfo * _Nonnull trackInfo, ElevationProfileData * _Nonnull (^ _Nullable elevationData) ()) { __strong __typeof(weakSelf) self = weakSelf; - + if (!self) return; switch (state) { case TrackRecordingStateInactive: [self stopObservingTrackRecordingUpdates]; diff --git a/iphone/Maps/Core/TrackRecorder/TrackRecordingManager.swift b/iphone/Maps/Core/TrackRecorder/TrackRecordingManager.swift index 797723004..7d253d5b8 100644 --- a/iphone/Maps/Core/TrackRecorder/TrackRecordingManager.swift +++ b/iphone/Maps/Core/TrackRecorder/TrackRecordingManager.swift @@ -9,15 +9,18 @@ enum TrackRecordingAction { case stopAndSave(name: String) } -enum TrackRecordingError: Error { +enum LocationError: Error { case locationIsProhibited - case trackIsEmpty - case systemError(Error) } -enum TrackRecordingActionResult { +enum StartTrackRecordingResult { case success - case error(TrackRecordingError) + case failure(Error) +} + +enum StopTrackRecordingResult { + case success + case trackIsEmpty } @objc @@ -44,8 +47,6 @@ typealias TrackRecordingStateHandler = (TrackRecordingState, TrackInfo, Elevatio @objcMembers final class TrackRecordingManager: NSObject { - typealias CompletionHandler = (TrackRecordingActionResult) -> Void - fileprivate struct Observation { weak var observer: AnyObject? var recordingStateDidChangeHandler: TrackRecordingStateHandler? @@ -111,22 +112,47 @@ final class TrackRecordingManager: NSObject { recordingState == .active } - func processAction(_ action: TrackRecordingAction, completion: (CompletionHandler)? = nil) { + func start(completion: ((StartTrackRecordingResult) -> Void)? = nil) { do { - switch action { - case .start: - try startRecording() - case .stopAndSave(let name): - stopRecording() - try checkIsTrackNotEmpty() - saveTrackRecording(name: name) + switch recordingState { + case .inactive: + try checkIsLocationEnabled() + subscribeOnTrackRecordingProgressUpdates() + trackRecorder.startTrackRecording() + notifyObservers() + do { + try activityManager?.start(with: trackRecordingInfo) + } catch { + LOG(.warning, "Failed to start activity manager") + handleError(error) + } + case .active: + break } completion?(.success) } catch { - handleError(error, completion: completion) + handleError(error) + completion?(.failure(error)) } } + func stopAndSave(withName name: String = "", completion: ((StopTrackRecordingResult) -> Void)? = nil) { + unsubscribeFromTrackRecordingProgressUpdates() + trackRecorder.stopTrackRecording() + trackRecordingInfo = .empty() + activityManager?.stop() + notifyObservers() + + guard !trackRecorder.isTrackRecordingEmpty() else { + Toast.show(withText: L("track_recording_toast_nothing_to_save")) + completion?(.trackIsEmpty) + return + } + + trackRecorder.saveTrackRecording(withName: name) + completion?(.success) + } + // MARK: - Private methods private func subscribeOnTheAppLifecycleEvents() { @@ -136,20 +162,12 @@ final class TrackRecordingManager: NSObject { object: nil) } - private func checkIsLocationEnabled() throws(TrackRecordingError) { + private func checkIsLocationEnabled() throws { if locationService.isLocationProhibited() { - throw TrackRecordingError.locationIsProhibited + throw LocationError.locationIsProhibited } } - private func checkIsTrackNotEmpty() throws(TrackRecordingError) { - if trackRecorder.isTrackRecordingEmpty() { - throw TrackRecordingError.trackIsEmpty - } - } - - // MARK: - Handle track recording process - private func subscribeOnTrackRecordingProgressUpdates() { trackRecorder.setTrackRecordingUpdateHandler { [weak self] info in guard let self else { return } @@ -163,52 +181,15 @@ final class TrackRecordingManager: NSObject { trackRecorder.setTrackRecordingUpdateHandler(nil) } - // MARK: - Handle Start/Stop event and Errors - - private func startRecording() throws(TrackRecordingError) { - switch recordingState { - case .inactive: - try checkIsLocationEnabled() - subscribeOnTrackRecordingProgressUpdates() - trackRecorder.startTrackRecording() - notifyObservers() - do { - try activityManager?.start(with: trackRecordingInfo) - } catch { - LOG(.warning, "Failed to start activity manager") - handleError(.systemError(error)) - } - case .active: - break - } - } - - private func stopRecording() { - unsubscribeFromTrackRecordingProgressUpdates() - trackRecorder.stopTrackRecording() - trackRecordingInfo = .empty() - activityManager?.stop() - notifyObservers() - } - - private func saveTrackRecording(name: String) { - trackRecorder.saveTrackRecording(withName: name) - } - - private func handleError(_ error: TrackRecordingError, completion: (CompletionHandler)? = nil) { + private func handleError(_ error: Error) { switch error { - case TrackRecordingError.locationIsProhibited: + case LocationError.locationIsProhibited: // Show alert to enable location locationService.checkLocationStatus() - case TrackRecordingError.trackIsEmpty: - Toast.show(withText: L("track_recording_toast_nothing_to_save")) - case TrackRecordingError.systemError(let error): + default: LOG(.error, error.localizedDescription) break } - DispatchQueue.main.async { - completion?(.error(error)) - } } } diff --git a/iphone/Maps/Tests/Core/TrackRecorder/TrackRecordingManagerTests.swift b/iphone/Maps/Tests/Core/TrackRecorder/TrackRecordingManagerTests.swift index ecb44380d..6ea3c5ee8 100644 --- a/iphone/Maps/Tests/Core/TrackRecorder/TrackRecordingManagerTests.swift +++ b/iphone/Maps/Tests/Core/TrackRecorder/TrackRecordingManagerTests.swift @@ -51,7 +51,7 @@ final class TrackRecordingManagerTests: XCTestCase { mockLocationService.locationIsProhibited = false mockTrackRecorder.trackRecordingIsEnabled = false - trackRecordingManager.processAction(.start) + trackRecordingManager.start() XCTAssertTrue(mockTrackRecorder.startTrackRecordingCalled) XCTAssertTrue(mockActivityManager.startCalled) @@ -60,20 +60,21 @@ final class TrackRecordingManagerTests: XCTestCase { func test_GivenStartRecording_WhenLocationDisabled_ThenShouldFail() { mockLocationService.locationIsProhibited = true - - trackRecordingManager.processAction(.start) { result in + let expectation = expectation(description: "Location is prohibited") + trackRecordingManager.start() { result in switch result { case .success: XCTFail("Should not succeed") - case .error(let error): + case .failure(let error): switch error { - case .locationIsProhibited: - XCTAssertTrue(true) + case LocationError.locationIsProhibited: + expectation.fulfill() default: XCTFail("Unexpected error: \(error)") } } } + wait(for: [expectation], timeout: 1.0) XCTAssertFalse(self.mockTrackRecorder.startTrackRecordingCalled) XCTAssertTrue(trackRecordingManager.recordingState == .inactive) } @@ -82,12 +83,12 @@ final class TrackRecordingManagerTests: XCTestCase { mockTrackRecorder.trackRecordingIsEnabled = true mockTrackRecorder.trackRecordingIsEmpty = false - trackRecordingManager.processAction(.stopAndSave(name: "Test Track")) { result in + trackRecordingManager.stopAndSave(withName: "Test Track") { result in switch result { case .success: XCTAssertTrue(true) - case .error(let error): - XCTFail("Unexpected error: \(error)") + case .trackIsEmpty: + XCTFail("Track should not be empty") } } XCTAssertTrue(mockTrackRecorder.stopTrackRecordingCalled) @@ -99,20 +100,16 @@ final class TrackRecordingManagerTests: XCTestCase { func test_GivenStopRecording_WhenTrackIsEmpty_ThenShouldFail() { mockTrackRecorder.trackRecordingIsEnabled = true mockTrackRecorder.trackRecordingIsEmpty = true - - trackRecordingManager.processAction(.stopAndSave(name: "Test Track")) { result in + let expectation = expectation(description: "Track recording should be empty") + trackRecordingManager.stopAndSave(withName: "Test Track") { result in switch result { case .success: XCTFail("Should not succeed") - case .error(let error): - switch error { - case .trackIsEmpty: - XCTAssertTrue(true) - default: - XCTFail("Unexpected error: \(error)") - } + case .trackIsEmpty: + expectation.fulfill() } } + wait(for: [expectation], timeout: 1.0) XCTAssertFalse(mockTrackRecorder.saveTrackRecordingCalled) XCTAssertTrue(trackRecordingManager.recordingState == .inactive) } diff --git a/iphone/Maps/UI/BottomMenu/Menu/BottomMenuInteractor.swift b/iphone/Maps/UI/BottomMenu/Menu/BottomMenuInteractor.swift index 3ec98b553..2d502d5e4 100644 --- a/iphone/Maps/UI/BottomMenu/Menu/BottomMenuInteractor.swift +++ b/iphone/Maps/UI/BottomMenu/Menu/BottomMenuInteractor.swift @@ -76,13 +76,19 @@ extension BottomMenuInteractor: BottomMenuInteractorProtocol { } func toggleTrackRecording() { + close() switch trackRecorder.recordingState { case .active: break case .inactive: - trackRecorder.processAction(.start) + trackRecorder.start { result in + switch result { + case .success: + MapViewController.shared()?.showTrackRecordingPlacePage() + case .failure: + break + } + } } - close() - MapViewController.shared()?.showTrackRecordingPlacePage() } } diff --git a/iphone/Maps/UI/PlacePage/Components/ElevationProfile/ElevationProfilePresenter.swift b/iphone/Maps/UI/PlacePage/Components/ElevationProfile/ElevationProfilePresenter.swift index 00abec743..7074e464c 100644 --- a/iphone/Maps/UI/PlacePage/Components/ElevationProfile/ElevationProfilePresenter.swift +++ b/iphone/Maps/UI/PlacePage/Components/ElevationProfile/ElevationProfilePresenter.swift @@ -76,12 +76,12 @@ extension ElevationProfilePresenter: ElevationProfilePresenterProtocol { } func configure() { + view?.isChartViewHidden = false + let kMinPointsToDraw = 3 guard let profileData, let chartData, chartData.points.count >= kMinPointsToDraw else { - view?.isChartViewHidden = true return } - view?.isChartViewHidden = false view?.setChartData(ChartPresentationData(chartData, formatter: formatter)) view?.reloadDescription() diff --git a/iphone/Maps/UI/PlacePage/PlacePageInteractor.swift b/iphone/Maps/UI/PlacePage/PlacePageInteractor.swift index c4accd9fa..5ca6e711a 100644 --- a/iphone/Maps/UI/PlacePage/PlacePageInteractor.swift +++ b/iphone/Maps/UI/PlacePage/PlacePageInteractor.swift @@ -251,11 +251,11 @@ extension PlacePageInteractor: ActionBarViewControllerDelegate { showTrackDeletionConfirmationDialog() case .saveTrackRecording: // TODO: (KK) pass name typed by user - TrackRecordingManager.shared.processAction(.stopAndSave(name: "")) { [weak self] result in + TrackRecordingManager.shared.stopAndSave() { [weak self] result in switch result { case .success: break - case .error: + case .trackIsEmpty: self?.presenter?.closeAnimated() } }