Skip to content
This repository was archived by the owner on Feb 24, 2025. It is now read-only.

Commit 5f65053

Browse files
committed
Fix invalid state transition when only isLoading is changed
1 parent b8f782f commit 5f65053

File tree

4 files changed

+138
-9
lines changed

4 files changed

+138
-9
lines changed

DuckDuckGo.xcodeproj/project.pbxproj

+4
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,7 @@
315315
6F64AA5F2C49463C00CF4489 /* ShortcutsModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6F64AA5E2C49463C00CF4489 /* ShortcutsModel.swift */; };
316316
6F655BE22BAB289E00AC3597 /* DefaultTheme.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6F655BE12BAB289E00AC3597 /* DefaultTheme.swift */; };
317317
6F691CCA2C4979EC002E9553 /* FavoritesTooltip.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6F691CC92C4979EC002E9553 /* FavoritesTooltip.swift */; };
318+
6F7BACD42CEE084B00F561D8 /* OmniBarEqualityCheckTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6F7BACD32CEE084100F561D8 /* OmniBarEqualityCheckTests.swift */; };
318319
6F7FB8E12C660B3E00867DA7 /* NewTabPageFavoritesModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6F7FB8DF2C660B1A00867DA7 /* NewTabPageFavoritesModelTests.swift */; };
319320
6F7FB8E32C660BF300867DA7 /* DailyPixelFiring.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6F7FB8E22C660BF300867DA7 /* DailyPixelFiring.swift */; };
320321
6F7FB8E52C66158D00867DA7 /* NewTabPageShortcutsSettingsModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6F7FB8E42C66158D00867DA7 /* NewTabPageShortcutsSettingsModelTests.swift */; };
@@ -1623,6 +1624,7 @@
16231624
6F64AA5E2C49463C00CF4489 /* ShortcutsModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShortcutsModel.swift; sourceTree = "<group>"; };
16241625
6F655BE12BAB289E00AC3597 /* DefaultTheme.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DefaultTheme.swift; sourceTree = "<group>"; };
16251626
6F691CC92C4979EC002E9553 /* FavoritesTooltip.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FavoritesTooltip.swift; sourceTree = "<group>"; };
1627+
6F7BACD32CEE084100F561D8 /* OmniBarEqualityCheckTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OmniBarEqualityCheckTests.swift; sourceTree = "<group>"; };
16261628
6F7FB8DF2C660B1A00867DA7 /* NewTabPageFavoritesModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NewTabPageFavoritesModelTests.swift; sourceTree = "<group>"; };
16271629
6F7FB8E22C660BF300867DA7 /* DailyPixelFiring.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DailyPixelFiring.swift; sourceTree = "<group>"; };
16281630
6F7FB8E42C66158D00867DA7 /* NewTabPageShortcutsSettingsModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NewTabPageShortcutsSettingsModelTests.swift; sourceTree = "<group>"; };
@@ -6348,6 +6350,7 @@
63486350
isa = PBXGroup;
63496351
children = (
63506352
6F3529FE2CDCEDF700A59170 /* OmniBarLoadingStateBearerTests.swift */,
6353+
6F7BACD32CEE084100F561D8 /* OmniBarEqualityCheckTests.swift */,
63516354
BBFF18B02C76448100C48D7D /* QuerySubmittedTests.swift */,
63526355
8588026424E4209900C24AB6 /* LargeOmniBarStateTests.swift */,
63536356
85F20005221702F7006BB258 /* AddressDisplayHelperTests.swift */,
@@ -8090,6 +8093,7 @@
80908093
85D2187224BF24F2004373D2 /* NotFoundCachingDownloaderTests.swift in Sources */,
80918094
C1935A242C89CC6D001AD72D /* AutofillHeaderViewFactoryTests.swift in Sources */,
80928095
C111B26927F579EF006558B1 /* BookmarkOrFolderTests.swift in Sources */,
8096+
6F7BACD42CEE084B00F561D8 /* OmniBarEqualityCheckTests.swift in Sources */,
80938097
6F7FB8E72C66197E00867DA7 /* NewTabPageSectionsSettingsModelTests.swift in Sources */,
80948098
851CD674244D7E6000331B98 /* UserDefaultsExtension.swift in Sources */,
80958099
569437362BE5160600C0881B /* SyncSettingsViewControllerErrorTests.swift in Sources */,

DuckDuckGo/OmniBar.swift

+9-5
Original file line numberDiff line numberDiff line change
@@ -360,15 +360,19 @@ class OmniBar: UIView {
360360
}
361361

362362
fileprivate func refreshState(_ newState: any OmniBarState) {
363-
if !newState.isEquivalent(to: state) {
363+
if state.requiresUpdate(transitioningInto: newState) {
364364
Logger.general.debug("OmniBar entering \(newState.description) from \(self.state.description)")
365-
if newState.clearTextOnStart {
366-
clear()
365+
366+
if state.isDifferentState(than: newState) {
367+
if newState.clearTextOnStart {
368+
clear()
369+
}
370+
cancelAllAnimations()
367371
}
372+
368373
state = newState
369-
cancelAllAnimations()
370374
}
371-
375+
372376
searchFieldContainer.adjustTextFieldOffset(for: state)
373377

374378
setVisibility(privacyInfoContainer, hidden: !state.showPrivacyIcon)

DuckDuckGo/OmniBarState.swift

+12-4
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ protocol OmniBarState: CustomStringConvertible {
2929
var showForwardButton: Bool { get }
3030
var showBookmarksButton: Bool { get }
3131
var showShareButton: Bool { get }
32-
32+
3333
var clearTextOnStart: Bool { get }
3434
var allowsTrackersAnimation: Bool { get }
3535
var showSearchLoupe: Bool { get }
@@ -61,12 +61,20 @@ protocol OmniBarState: CustomStringConvertible {
6161
func withLoading() -> Self
6262
func withoutLoading() -> Self
6363

64-
func isEquivalent(to other: OmniBarState) -> Bool
64+
func requiresUpdate(transitioningInto other: OmniBarState) -> Bool
65+
func isDifferentState(than other: OmniBarState) -> Bool
6566
}
6667

6768
extension OmniBarState {
68-
func isEquivalent(to other: OmniBarState) -> Bool {
69-
name == other.name && isLoading == other.isLoading
69+
/// Returns if new state requires UI update
70+
func requiresUpdate(transitioningInto other: OmniBarState) -> Bool {
71+
name != other.name || isLoading != other.isLoading
72+
}
73+
74+
/// Checks whether the state type is different.
75+
/// If `true` it may require transitioning to a different appearance and/or cancelling pending animations.
76+
func isDifferentState(than other: OmniBarState) -> Bool {
77+
name != other.name
7078
}
7179

7280
var description: String {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
//
2+
// OmniBarEqualityCheckTests.swift
3+
// DuckDuckGo
4+
//
5+
// Copyright © 2024 DuckDuckGo. All rights reserved.
6+
//
7+
// Licensed under the Apache License, Version 2.0 (the "License");
8+
// you may not use this file except in compliance with the License.
9+
// You may obtain a copy of the License at
10+
//
11+
// http://www.apache.org/licenses/LICENSE-2.0
12+
//
13+
// Unless required by applicable law or agreed to in writing, software
14+
// distributed under the License is distributed on an "AS IS" BASIS,
15+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
// See the License for the specific language governing permissions and
17+
// limitations under the License.
18+
//
19+
20+
import XCTest
21+
@testable import DuckDuckGo
22+
23+
final class OmniBarEqualityCheckTests: XCTestCase {
24+
func testRequiresUpdateChecksForIsLoading() {
25+
let loadingOmniBarState = DummyOmniBarState(isLoading: true)
26+
let notLoadingOmniBarState = DummyOmniBarState(isLoading: false)
27+
28+
XCTAssertTrue(loadingOmniBarState.requiresUpdate(transitioningInto: notLoadingOmniBarState))
29+
}
30+
31+
func testRequiresUpdateChecksForName() {
32+
let fooOmniBarState = DummyOmniBarState(name: "foo")
33+
let barOmniBarState = DummyOmniBarState(name: "bar")
34+
35+
XCTAssertTrue(fooOmniBarState.requiresUpdate(transitioningInto: barOmniBarState))
36+
}
37+
38+
func testIsDifferentStateChecksForName() {
39+
let fooOmniBarState = DummyOmniBarState(name: "foo")
40+
let barOmniBarState = DummyOmniBarState(name: "bar")
41+
42+
XCTAssertTrue(fooOmniBarState.isDifferentState(than: barOmniBarState))
43+
}
44+
45+
func testIsDifferentStateIgnoresOtherProperties() {
46+
let fooOmniBarState = DummyOmniBarState()
47+
var barOmniBarState = DummyOmniBarState()
48+
49+
barOmniBarState.hasLargeWidth = !fooOmniBarState.hasLargeWidth
50+
barOmniBarState.showBackButton = !fooOmniBarState.showBackButton
51+
barOmniBarState.showForwardButton = !fooOmniBarState.showForwardButton
52+
barOmniBarState.showBookmarksButton = !fooOmniBarState.showBookmarksButton
53+
barOmniBarState.showShareButton = !fooOmniBarState.showShareButton
54+
barOmniBarState.clearTextOnStart = !fooOmniBarState.clearTextOnStart
55+
barOmniBarState.allowsTrackersAnimation = !fooOmniBarState.allowsTrackersAnimation
56+
barOmniBarState.showSearchLoupe = !fooOmniBarState.showSearchLoupe
57+
barOmniBarState.showCancel = !fooOmniBarState.showCancel
58+
barOmniBarState.showPrivacyIcon = !fooOmniBarState.showPrivacyIcon
59+
barOmniBarState.showBackground = !fooOmniBarState.showBackground
60+
barOmniBarState.showClear = !fooOmniBarState.showClear
61+
barOmniBarState.showRefresh = !fooOmniBarState.showRefresh
62+
barOmniBarState.showMenu = !fooOmniBarState.showMenu
63+
barOmniBarState.showSettings = !fooOmniBarState.showSettings
64+
barOmniBarState.showVoiceSearch = !fooOmniBarState.showVoiceSearch
65+
barOmniBarState.showAbort = !fooOmniBarState.showAbort
66+
67+
XCTAssertFalse(fooOmniBarState.isDifferentState(than: barOmniBarState))
68+
}
69+
}
70+
71+
private struct DummyOmniBarState: OmniBarState, OmniBarLoadingBearerStateCreating {
72+
var name: String
73+
var isLoading: Bool
74+
var voiceSearchHelper: VoiceSearchHelperProtocol
75+
76+
var hasLargeWidth = false
77+
var showBackButton = false
78+
var showForwardButton = false
79+
var showBookmarksButton = false
80+
var showShareButton = false
81+
var clearTextOnStart = false
82+
var allowsTrackersAnimation = false
83+
var showSearchLoupe = false
84+
var showCancel = false
85+
var showPrivacyIcon = false
86+
var showBackground = false
87+
var showClear = false
88+
var showRefresh = false
89+
var showMenu = false
90+
var showSettings = false
91+
var showVoiceSearch = false
92+
var showAbort = false
93+
94+
var onEditingStoppedState: OmniBarState { DummyOmniBarState() }
95+
var onEditingStartedState: OmniBarState { DummyOmniBarState() }
96+
var onTextClearedState: OmniBarState { DummyOmniBarState() }
97+
var onTextEnteredState: OmniBarState { DummyOmniBarState() }
98+
var onBrowsingStartedState: OmniBarState { DummyOmniBarState() }
99+
var onBrowsingStoppedState: OmniBarState { DummyOmniBarState() }
100+
var onEnterPhoneState: OmniBarState { DummyOmniBarState() }
101+
var onEnterPadState: OmniBarState { DummyOmniBarState() }
102+
var onReloadState: OmniBarState { DummyOmniBarState() }
103+
104+
init(voiceSearchHelper: VoiceSearchHelperProtocol, isLoading: Bool) {
105+
self.init(isLoading: isLoading, voiceSearchHelper: voiceSearchHelper)
106+
}
107+
108+
init(name: String = "DummyOmniBarState", isLoading: Bool = false, voiceSearchHelper: VoiceSearchHelperProtocol = MockVoiceSearchHelper()) {
109+
self.name = name
110+
self.isLoading = isLoading
111+
self.voiceSearchHelper = voiceSearchHelper
112+
}
113+
}

0 commit comments

Comments
 (0)