Skip to content

Commit

Permalink
Fix bookmarks bar visibility (#2554)
Browse files Browse the repository at this point in the history
Task/Issue URL:
https://app.asana.com/0/1177771139624306/1207002191511983/f

**Description**:
The current bookmarks bar has some bugs related to its visibility when the appearance preference is “Only shows on New Tab”. This PR extracts and fix the logic to determine whether the bookmarks bar should be visible or not.
  • Loading branch information
alessandroboron authored Apr 8, 2024
1 parent 6483649 commit d0fb45d
Show file tree
Hide file tree
Showing 5 changed files with 418 additions and 18 deletions.
14 changes: 14 additions & 0 deletions DuckDuckGo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -2479,6 +2479,11 @@
9F26060C2B85C20B00819292 /* AddEditBookmarkDialogViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F2606092B85C20400819292 /* AddEditBookmarkDialogViewModelTests.swift */; };
9F26060E2B85E17D00819292 /* AddEditBookmarkDialogCoordinatorViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F26060D2B85E17D00819292 /* AddEditBookmarkDialogCoordinatorViewModelTests.swift */; };
9F26060F2B85E17D00819292 /* AddEditBookmarkDialogCoordinatorViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F26060D2B85E17D00819292 /* AddEditBookmarkDialogCoordinatorViewModelTests.swift */; };
9F33445E2BBFA77F0040CBEB /* BookmarksBarVisibilityManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F33445D2BBFA77F0040CBEB /* BookmarksBarVisibilityManager.swift */; };
9F33445F2BBFA77F0040CBEB /* BookmarksBarVisibilityManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F33445D2BBFA77F0040CBEB /* BookmarksBarVisibilityManager.swift */; };
9F3344602BBFA77F0040CBEB /* BookmarksBarVisibilityManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F33445D2BBFA77F0040CBEB /* BookmarksBarVisibilityManager.swift */; };
9F3344622BBFBDA40040CBEB /* BookmarksBarVisibilityManagerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F3344612BBFBDA40040CBEB /* BookmarksBarVisibilityManagerTests.swift */; };
9F3344632BBFBDA40040CBEB /* BookmarksBarVisibilityManagerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F3344612BBFBDA40040CBEB /* BookmarksBarVisibilityManagerTests.swift */; };
9F3910622B68C35600CB5112 /* DownloadsTabExtensionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F3910612B68C35600CB5112 /* DownloadsTabExtensionTests.swift */; };
9F3910632B68C35600CB5112 /* DownloadsTabExtensionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F3910612B68C35600CB5112 /* DownloadsTabExtensionTests.swift */; };
9F3910692B68D87B00CB5112 /* ProgressExtensionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F3910682B68D87B00CB5112 /* ProgressExtensionTests.swift */; };
Expand Down Expand Up @@ -4227,6 +4232,8 @@
9F180D112B69C665000D695F /* DownloadsTabExtensionMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DownloadsTabExtensionMock.swift; sourceTree = "<group>"; };
9F2606092B85C20400819292 /* AddEditBookmarkDialogViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AddEditBookmarkDialogViewModelTests.swift; sourceTree = "<group>"; };
9F26060D2B85E17D00819292 /* AddEditBookmarkDialogCoordinatorViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AddEditBookmarkDialogCoordinatorViewModelTests.swift; sourceTree = "<group>"; };
9F33445D2BBFA77F0040CBEB /* BookmarksBarVisibilityManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarksBarVisibilityManager.swift; sourceTree = "<group>"; };
9F3344612BBFBDA40040CBEB /* BookmarksBarVisibilityManagerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarksBarVisibilityManagerTests.swift; sourceTree = "<group>"; };
9F3910612B68C35600CB5112 /* DownloadsTabExtensionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DownloadsTabExtensionTests.swift; sourceTree = "<group>"; };
9F3910682B68D87B00CB5112 /* ProgressExtensionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProgressExtensionTests.swift; sourceTree = "<group>"; };
9F514F902B7D88AD001832A9 /* AddEditBookmarkFolderDialogView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AddEditBookmarkFolderDialogView.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -5761,6 +5768,7 @@
isa = PBXGroup;
children = (
4B43468E285ED6CB00177407 /* ViewModel */,
9F3344612BBFBDA40040CBEB /* BookmarksBarVisibilityManagerTests.swift */,
);
path = BookmarksBar;
sourceTree = "<group>";
Expand Down Expand Up @@ -6481,6 +6489,7 @@
children = (
4BD18F02283F0F1000058124 /* View */,
850E8DFA2A6FEC5E00691187 /* BookmarksBarAppearance.swift */,
9F33445D2BBFA77F0040CBEB /* BookmarksBarVisibilityManager.swift */,
);
path = BookmarksBar;
sourceTree = "<group>";
Expand Down Expand Up @@ -10528,6 +10537,7 @@
4B9DB0422A983B24000927DB /* WaitlistDialogView.swift in Sources */,
3706FB57293F65D500E42796 /* AppPrivacyConfigurationDataProvider.swift in Sources */,
857E5AF62A790B7000FC0FB4 /* PixelExperiment.swift in Sources */,
9F33445F2BBFA77F0040CBEB /* BookmarksBarVisibilityManager.swift in Sources */,
3706FB58293F65D500E42796 /* LinkButton.swift in Sources */,
4B0EF7272B578096009D6481 /* AppVersionExtension.swift in Sources */,
3706FB59293F65D500E42796 /* TemporaryFileHandler.swift in Sources */,
Expand Down Expand Up @@ -11231,6 +11241,7 @@
028904212A7B25770028369C /* AppConfigurationURLProviderTests.swift in Sources */,
3706FE6F293F661700E42796 /* LocalStatisticsStoreTests.swift in Sources */,
3706FE70293F661700E42796 /* HistoryCoordinatorTests.swift in Sources */,
9F3344632BBFBDA40040CBEB /* BookmarksBarVisibilityManagerTests.swift in Sources */,
3706FE71293F661700E42796 /* SavedStateMock.swift in Sources */,
3706FE72293F661700E42796 /* ClickToLoadTDSTests.swift in Sources */,
3706FE73293F661700E42796 /* PermissionManagerMock.swift in Sources */,
Expand Down Expand Up @@ -11879,6 +11890,7 @@
4B957A932AC7AE700062CA31 /* PermissionModel.swift in Sources */,
4B957A942AC7AE700062CA31 /* PasteboardFolder.swift in Sources */,
4B957A952AC7AE700062CA31 /* CookieManagedNotificationView.swift in Sources */,
9F3344602BBFA77F0040CBEB /* BookmarksBarVisibilityManager.swift in Sources */,
4B957A962AC7AE700062CA31 /* PermissionType.swift in Sources */,
4B957A982AC7AE700062CA31 /* RecentlyClosedWindow.swift in Sources */,
4B957A992AC7AE700062CA31 /* ActionSpeech.swift in Sources */,
Expand Down Expand Up @@ -12958,6 +12970,7 @@
AA7EB6DF27E7C57D00036718 /* MouseOverAnimationButton.swift in Sources */,
AA7412B724D1687000D22FE0 /* TabBarScrollView.swift in Sources */,
1E559BB12BBCA9F1002B4AF6 /* RedirectNavigationResponder.swift in Sources */,
9F33445E2BBFA77F0040CBEB /* BookmarksBarVisibilityManager.swift in Sources */,
4B9292D92667124B00AD2C21 /* BookmarkListTreeControllerDataSource.swift in Sources */,
14D9B8FB24F7E089000D4D13 /* AddressBarViewController.swift in Sources */,
B65536A62685B82B00085A79 /* Permissions.swift in Sources */,
Expand Down Expand Up @@ -13136,6 +13149,7 @@
AAC9C01C24CB594C00AD1325 /* TabViewModelTests.swift in Sources */,
567DA93F29E8045D008AC5EE /* MockEmailStorage.swift in Sources */,
37CD54B727F1B28A00F1F7B9 /* DefaultBrowserPreferencesTests.swift in Sources */,
9F3344622BBFBDA40040CBEB /* BookmarksBarVisibilityManagerTests.swift in Sources */,
028904202A7B25380028369C /* AppConfigurationURLProviderTests.swift in Sources */,
B65349AA265CF45000DCC645 /* DispatchQueueExtensionsTests.swift in Sources */,
858A798A26A9B35E00A75A42 /* PasswordManagementItemModelTests.swift in Sources */,
Expand Down
96 changes: 96 additions & 0 deletions DuckDuckGo/BookmarksBar/BookmarksBarVisibilityManager.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
//
// BookmarksBarVisibilityManager.swift
//
// Copyright © 2024 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Foundation
import Combine

/// Decides if the BookmarksBar should be visible based on the Tab.Content and Appearance preferences.
final class BookmarksBarVisibilityManager {
private var bookmarkBarVisibilityCancellable: AnyCancellable?
private var bookmarkContentCancellable: AnyCancellable?

/// A published value indicating the visibility of the bookmarks bar.
/// Returns`true` if the bookmarks bar is visible; otherwise, `false`.
@Published var isBookmarksBarVisible: Bool = false

private let selectedTabPublisher: AnyPublisher<TabViewModel?, Never>
private let preferences: AppearancePreferences

/// Create an instance given the specified `TabViewModel` publisher and `AppearancePreferences`.
/// - Parameters:
/// - selectedTabPublisher: A publisher that returns the selected Tab view model.
/// - preferences: The `AppearancePreferences` to read the bookmarks appearance preferences from.
init(selectedTabPublisher: AnyPublisher<TabViewModel?, Never>, preferences: AppearancePreferences = .shared) {
self.selectedTabPublisher = selectedTabPublisher
self.preferences = preferences
bind()
}

}

// MARK: - Private

private extension BookmarksBarVisibilityManager {

func bind() {
let bookmarksBarVisibilityPublisher = NotificationCenter.default
.publisher(for: AppearancePreferences.Notifications.showBookmarksBarSettingChanged)

let bookmarksBarAppearancePublisher = NotificationCenter.default
.publisher(for: AppearancePreferences.Notifications.bookmarksBarSettingAppearanceChanged)

let bookmarksBarNotificationsPublisher = Publishers.Merge(bookmarksBarVisibilityPublisher, bookmarksBarAppearancePublisher)
.map { _ in () } // Map To Void, we're not interested in the notification itself
.prepend(()) // Start with a value so combineLatest can fire

// Every time the user select a tab or the Appeareance preference changes check if bookmarks bar should be visible or not.
// For the selected Tab we should also check if the Tab content changes as it can switch from empty to url if the user loads a web page.
bookmarkBarVisibilityCancellable = bookmarksBarNotificationsPublisher
.combineLatest(selectedTabPublisher)
.compactMap { _, selectedTab -> TabViewModel? in
guard let selectedTab else { return nil }
return selectedTab
}
.flatMap { tabViewModel in
// Subscribe to the selected tab content.
// When a tab is empty and the bookmarksBar should show only on empty Tabs it should disappear when we load a website.
tabViewModel.tab.$content.eraseToAnyPublisher()
}
.sink(receiveValue: { [weak self] tabContent in
guard let self = self else { return }
self.updateBookmarksBar(content: tabContent, preferences: self.preferences)
})
}

func updateBookmarksBar(content: Tab.TabContent, preferences: AppearancePreferences) {
// If visibility should be off, set visibility off and exit
guard preferences.showBookmarksBar else {
isBookmarksBarVisible = false
return
}

// If visibility is on check Appearance
switch preferences.bookmarksBarAppearance {
case .newTabOnly:
isBookmarksBarVisible = content.isEmpty
case .alwaysOn:
isBookmarksBarVisible = true
}
}

}
28 changes: 10 additions & 18 deletions DuckDuckGo/MainWindow/MainViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ final class MainViewController: NSViewController {
let findInPageViewController: FindInPageViewController
let fireViewController: FireViewController
let bookmarksBarViewController: BookmarksBarViewController
private let bookmarksBarVisibilityManager: BookmarksBarVisibilityManager

let tabCollectionViewModel: TabCollectionViewModel
let isBurner: Bool
Expand Down Expand Up @@ -60,6 +61,7 @@ final class MainViewController: NSViewController {
self.isBurner = tabCollectionViewModel.isBurner

tabBarViewController = TabBarViewController.create(tabCollectionViewModel: tabCollectionViewModel)
bookmarksBarVisibilityManager = BookmarksBarVisibilityManager(selectedTabPublisher: tabCollectionViewModel.$selectedTabViewModel.eraseToAnyPublisher())

let networkProtectionPopoverManager: NetPPopoverManager = {
#if DEBUG
Expand Down Expand Up @@ -124,7 +126,7 @@ final class MainViewController: NSViewController {
listenToKeyDownEvents()
subscribeToMouseTrackingArea()
subscribeToSelectedTabViewModel()
subscribeToAppSettingsNotifications()
subscribeToBookmarkBarVisibility()
subscribeToFirstResponder()
mainView.findInPageContainerView.applyDropShadow()

Expand Down Expand Up @@ -171,9 +173,6 @@ final class MainViewController: NSViewController {

resizeNavigationBar(isHomePage: tabCollectionViewModel.selectedTabViewModel?.tab.content == .newtab,
animated: false)

let bookmarksBarVisible = AppearancePreferences.shared.showBookmarksBar
updateBookmarksBarViewVisibility(visible: bookmarksBarVisible)
}

updateDividerColor(isShowingHomePage: tabCollectionViewModel.selectedTabViewModel?.tab.content == .newtab)
Expand Down Expand Up @@ -314,11 +313,13 @@ final class MainViewController: NSViewController {
.store(in: &tabViewModelCancellables)
}

private func subscribeToAppSettingsNotifications() {
bookmarksBarVisibilityChangedCancellable = NotificationCenter.default
.publisher(for: AppearancePreferences.Notifications.showBookmarksBarSettingChanged)
.sink { [weak self] _ in
self?.updateBookmarksBarViewVisibility(visible: AppearancePreferences.shared.showBookmarksBar)
private func subscribeToBookmarkBarVisibility() {
bookmarksBarVisibilityChangedCancellable = bookmarksBarVisibilityManager
.$isBookmarksBarVisible
.removeDuplicates()
.receive(on: DispatchQueue.main)
.sink { [weak self] isBookmarksBarVisible in
self?.updateBookmarksBarViewVisibility(visible: isBookmarksBarVisible)
}
}

Expand All @@ -335,7 +336,6 @@ final class MainViewController: NSViewController {
defer { lastTabContent = content }

resizeNavigationBar(isHomePage: content == .newtab, animated: content == .newtab && lastTabContent != .newtab)
updateBookmarksBar(content)
adjustFirstResponder(selectedTabViewModel: selectedTabViewModel, tabContent: content)
}
.store(in: &self.tabViewModelCancellables)
Expand All @@ -355,14 +355,6 @@ final class MainViewController: NSViewController {
}
}

private func updateBookmarksBar(_ content: Tab.TabContent, _ prefs: AppearancePreferences = AppearancePreferences.shared) {
if content.isUrl && prefs.bookmarksBarAppearance == .newTabOnly {
updateBookmarksBarViewVisibility(visible: false)
} else if prefs.showBookmarksBar {
updateBookmarksBarViewVisibility(visible: true)
}
}

private func subscribeToFindInPage(of selectedTabViewModel: TabViewModel?) {
selectedTabViewModel?.findInPage?
.$isVisible
Expand Down
2 changes: 2 additions & 0 deletions DuckDuckGo/Preferences/Model/AppearancePreferences.swift
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ final class AppearancePreferences: ObservableObject {

struct Notifications {
static let showBookmarksBarSettingChanged = NSNotification.Name("ShowBookmarksBarSettingChanged")
static let bookmarksBarSettingAppearanceChanged = NSNotification.Name("BookmarksBarSettingAppearanceChanged")
}

static let shared = AppearancePreferences()
Expand Down Expand Up @@ -197,6 +198,7 @@ final class AppearancePreferences: ObservableObject {
@Published var bookmarksBarAppearance: BookmarksBarAppearance {
didSet {
persistor.bookmarksBarAppearance = bookmarksBarAppearance
NotificationCenter.default.post(name: Notifications.bookmarksBarSettingAppearanceChanged, object: nil)
}
}

Expand Down
Loading

0 comments on commit d0fb45d

Please sign in to comment.