diff --git a/DuckDuckGo/BarsAnimator.swift b/DuckDuckGo/BarsAnimator.swift index 01cae59724..c52e7201d7 100644 --- a/DuckDuckGo/BarsAnimator.swift +++ b/DuckDuckGo/BarsAnimator.swift @@ -87,18 +87,38 @@ class BarsAnimator { } private func transitioningAndScrolling(in scrollView: UIScrollView) { - let ratio = calculateTransitionRatio(for: scrollView.contentOffset.y) + + // On iOS 18 we end up in a loop after setBarsVisibility. + // It seems to trigger a new didScrollEvent when rendering some PDF files + // That causes an infinite loop. + // Are viewDidScroll calls happening more often for PDF's on iOS 18? + // Adding a debouncer while we investigate further + // https://app.asana.com/0/1204099484721401/1208671955053442/f + let debounceDelay: TimeInterval = 0.01 + struct Debounce { + static var workItem: DispatchWorkItem? + } + Debounce.workItem?.cancel() - if ratio == 1.0 { - barsState = .hidden - } else if ratio == 0 { - barsState = .revealed - } else if transitionProgress == ratio { - return + Debounce.workItem = DispatchWorkItem { [weak self] in + guard let self = self else { return } + + let ratio = self.calculateTransitionRatio(for: scrollView.contentOffset.y) + + if ratio == 1.0 { + self.barsState = .hidden + } else if ratio == 0 { + self.barsState = .revealed + } else if self.transitionProgress == ratio { + return + } + + self.delegate?.setBarsVisibility(1.0 - ratio, animated: false) + self.transitionProgress = ratio } - delegate?.setBarsVisibility(1.0 - ratio, animated: false) - transitionProgress = ratio + // Schedule the work item + DispatchQueue.main.asyncAfter(deadline: .now() + debounceDelay, execute: Debounce.workItem!) } private func hiddenAndScrolling(in scrollView: UIScrollView) { diff --git a/DuckDuckGoTests/BarsAnimatorTests.swift b/DuckDuckGoTests/BarsAnimatorTests.swift index 566d0099aa..cbbd8da93f 100644 --- a/DuckDuckGoTests/BarsAnimatorTests.swift +++ b/DuckDuckGoTests/BarsAnimatorTests.swift @@ -51,10 +51,15 @@ class BarsAnimatorTests: XCTestCase { scrollView.contentOffset.y = 300 sut.didScroll(in: scrollView) - XCTAssertEqual(sut.barsState, .hidden) - - XCTAssertEqual(delegate.receivedMessages, [.setBarsVisibility(0.0), - .setBarsVisibility(0.0)]) + + let expectation = XCTestExpectation(description: "Wait for bars state to update to hidden") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { + XCTAssertEqual(sut.barsState, .hidden) + XCTAssertEqual(delegate.receivedMessages, [.setBarsVisibility(0.0), .setBarsVisibility(0.0)]) + expectation.fulfill() + } + + wait(for: [expectation], timeout: 0.2) } func testBarStateHiddenWhenScrollDownKeepsHiddenState() { @@ -71,14 +76,28 @@ class BarsAnimatorTests: XCTestCase { scrollView.contentOffset.y = 300 sut.didScroll(in: scrollView) - XCTAssertEqual(sut.barsState, .hidden) + + // Add delay before checking hidden state + let expectation1 = XCTestExpectation(description: "Wait for bars state to update to hidden") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { + XCTAssertEqual(sut.barsState, .hidden) + expectation1.fulfill() + } + + wait(for: [expectation1], timeout: 0.2) scrollView.contentOffset.y = 100 sut.didScroll(in: scrollView) - XCTAssertEqual(sut.barsState, .hidden) - XCTAssertEqual(delegate.receivedMessages, [.setBarsVisibility(0.0), - .setBarsVisibility(0.0)]) + // Add another delay before checking that barsState remains hidden + let expectation2 = XCTestExpectation(description: "Wait to confirm bars state remains hidden") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { + XCTAssertEqual(sut.barsState, .hidden) + XCTAssertEqual(delegate.receivedMessages, [.setBarsVisibility(0.0), .setBarsVisibility(0.0)]) + expectation2.fulfill() + } + + wait(for: [expectation2], timeout: 0.2) } func testBarStateHiddenWhenScrollUpUpdatesToRevealedState() { @@ -95,7 +114,15 @@ class BarsAnimatorTests: XCTestCase { scrollView.contentOffset.y = 400 sut.didScroll(in: scrollView) - XCTAssertEqual(sut.barsState, .hidden) + + // Add delay before checking hidden state + let expectation1 = XCTestExpectation(description: "Wait for bars state to update to hidden") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { + XCTAssertEqual(sut.barsState, .hidden) + expectation1.fulfill() + } + + wait(for: [expectation1], timeout: 0.2) scrollView.contentOffset.y = -100 sut.didStartScrolling(in: scrollView) @@ -104,14 +131,22 @@ class BarsAnimatorTests: XCTestCase { scrollView.contentOffset.y = -150 sut.didScroll(in: scrollView) - XCTAssertEqual(sut.barsState, .revealed) - XCTAssertEqual(delegate.receivedMessages, [.setBarsVisibility(0.0), - .setBarsVisibility(0.0), - .setBarsVisibility(1.0), - .setBarsVisibility(1.0)]) + // Add another delay before checking revealed state + let expectation2 = XCTestExpectation(description: "Wait for bars state to update to revealed") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { + XCTAssertEqual(sut.barsState, .revealed) + XCTAssertEqual(delegate.receivedMessages, [.setBarsVisibility(0.0), + .setBarsVisibility(0.0), + .setBarsVisibility(1.0), + .setBarsVisibility(1.0)]) + expectation2.fulfill() + } + + wait(for: [expectation2], timeout: 0.2) } + func testBarStateRevealedWhenScrollUpDoNotChangeCurrentState() { let (sut, delegate) = makeSUT() let scrollView = mockScrollView()