Skip to content

Commit

Permalink
fix: Bookmark menu items were always disabled (#465)
Browse files Browse the repository at this point in the history
The way we were injecting the selection state meant that the bookmark
menu items never updated and were always disabled. This change removes
some misguided attempts at managing enable/disable for menu items and
moves the ownership of the selection model.
  • Loading branch information
jbmorley authored May 26, 2023
1 parent 929cc31 commit ff922ca
Show file tree
Hide file tree
Showing 12 changed files with 30 additions and 167 deletions.
8 changes: 0 additions & 8 deletions macos/Bookmarks-macOS.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
objects = {

/* Begin PBXBuildFile section */
D80710EB26C4847D007272C0 /* BookmarkDebugCommands.swift in Sources */ = {isa = PBXBuildFile; fileRef = D80710EA26C4847D007272C0 /* BookmarkDebugCommands.swift */; };
D80710F326C4B855007272C0 /* MainWindow.swift in Sources */ = {isa = PBXBuildFile; fileRef = D80710F226C4B855007272C0 /* MainWindow.swift */; };
D8164D6626C08DD500756D43 /* EditView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D8164D6526C08DD500756D43 /* EditView.swift */; };
D8164D6826C08EEC00756D43 /* BorderedSelection.swift in Sources */ = {isa = PBXBuildFile; fileRef = D8164D6726C08EEC00756D43 /* BorderedSelection.swift */; };
Expand All @@ -19,7 +18,6 @@
D861257526AA048E00EB5FEF /* RenameTagView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D861257426AA048E00EB5FEF /* RenameTagView.swift */; };
D879448F26CB4A5400D97155 /* MenuType.swift in Sources */ = {isa = PBXBuildFile; fileRef = D879448E26CB4A5400D97155 /* MenuType.swift */; };
D879449126CB57BE00D97155 /* ContextAwareKeyboardShortcut.swift in Sources */ = {isa = PBXBuildFile; fileRef = D879449026CB57BE00D97155 /* ContextAwareKeyboardShortcut.swift */; };
D879449326CB5E1900D97155 /* MainMenuItemCondition.swift in Sources */ = {isa = PBXBuildFile; fileRef = D879449226CB5E1900D97155 /* MainMenuItemCondition.swift */; };
D879449526CB5E3D00D97155 /* Countable.swift in Sources */ = {isa = PBXBuildFile; fileRef = D879449426CB5E3D00D97155 /* Countable.swift */; };
D891C454261E32F90024E1A6 /* BookmarksApp.swift in Sources */ = {isa = PBXBuildFile; fileRef = D891C453261E32F90024E1A6 /* BookmarksApp.swift */; };
D891C456261E32F90024E1A6 /* ContentView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D891C455261E32F90024E1A6 /* ContentView.swift */; };
Expand Down Expand Up @@ -68,7 +66,6 @@
/* End PBXContainerItemProxy section */

/* Begin PBXFileReference section */
D80710EA26C4847D007272C0 /* BookmarkDebugCommands.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkDebugCommands.swift; sourceTree = "<group>"; };
D80710F226C4B855007272C0 /* MainWindow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MainWindow.swift; sourceTree = "<group>"; };
D8164D6526C08DD500756D43 /* EditView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EditView.swift; sourceTree = "<group>"; };
D8164D6726C08EEC00756D43 /* BorderedSelection.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BorderedSelection.swift; sourceTree = "<group>"; };
Expand All @@ -80,7 +77,6 @@
D861257426AA048E00EB5FEF /* RenameTagView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RenameTagView.swift; sourceTree = "<group>"; };
D879448E26CB4A5400D97155 /* MenuType.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MenuType.swift; sourceTree = "<group>"; };
D879449026CB57BE00D97155 /* ContextAwareKeyboardShortcut.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContextAwareKeyboardShortcut.swift; sourceTree = "<group>"; };
D879449226CB5E1900D97155 /* MainMenuItemCondition.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MainMenuItemCondition.swift; sourceTree = "<group>"; };
D879449426CB5E3D00D97155 /* Countable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Countable.swift; sourceTree = "<group>"; };
D891C450261E32F90024E1A6 /* Bookmarks.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = Bookmarks.app; sourceTree = BUILT_PRODUCTS_DIR; };
D891C453261E32F90024E1A6 /* BookmarksApp.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarksApp.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -237,7 +233,6 @@
isa = PBXGroup;
children = (
D879449026CB57BE00D97155 /* ContextAwareKeyboardShortcut.swift */,
D879449226CB5E1900D97155 /* MainMenuItemCondition.swift */,
D8931F6326C5E16200A8021E /* SelectionSheetHandler.swift */,
);
path = Modifiers;
Expand All @@ -256,7 +251,6 @@
isa = PBXGroup;
children = (
D8A06F2028A7E92F0002A56D /* AccountCommands.swift */,
D80710EA26C4847D007272C0 /* BookmarkDebugCommands.swift */,
D8A336FD26B56AF700436200 /* BookmarkDestructiveCommands.swift */,
D8A3370226B56C7400436200 /* BookmarkEditCommands.swift */,
D8A336FF26B56C1B00436200 /* BookmarkOpenCommands.swift */,
Expand Down Expand Up @@ -464,7 +458,6 @@
D891C456261E32F90024E1A6 /* ContentView.swift in Sources */,
D835072A294A0DB40044ADE0 /* TagEditorModel.swift in Sources */,
D861257526AA048E00EB5FEF /* RenameTagView.swift in Sources */,
D80710EB26C4847D007272C0 /* BookmarkDebugCommands.swift in Sources */,
D8A336FE26B56AF700436200 /* BookmarkDestructiveCommands.swift in Sources */,
D8931F6426C5E16200A8021E /* SelectionSheetHandler.swift in Sources */,
D8A336FA26B569C200436200 /* BookmarkTagCommands.swift in Sources */,
Expand All @@ -480,7 +473,6 @@
D8164D6626C08DD500756D43 /* EditView.swift in Sources */,
D8A8001D2A1F76200042C03B /* View.swift in Sources */,
D8F3F28F26EE776900BFA43B /* LogInView.swift in Sources */,
D879449326CB5E1900D97155 /* MainMenuItemCondition.swift in Sources */,
D879449126CB57BE00D97155 /* ContextAwareKeyboardShortcut.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
Expand Down
13 changes: 6 additions & 7 deletions macos/Bookmarks/BookmarksApp.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,12 @@ struct BookmarksApp: App {

@Environment(\.manager) var manager

@StateObject var selection = BookmarksSelection()
@FocusedObject var selection: BookmarksSelection?

var body: some Scene {

WindowGroup {
MainWindow(manager: manager)
.environment(\.selection, selection)
}
.commands {
SidebarCommands()
Expand All @@ -49,15 +48,15 @@ struct BookmarksApp: App {
.keyboardShortcut("r", modifiers: .command)
}
CommandMenu("Bookmark") {
BookmarkOpenCommands(selection: selection)
BookmarkOpenCommands(selection: selection ?? BookmarksSelection())
.trailingDivider()
BookmarkDesctructiveCommands(selection: selection)
BookmarkDesctructiveCommands(selection: selection ?? BookmarksSelection())
.trailingDivider()
BookmarkEditCommands(selection: selection)
BookmarkEditCommands(selection: selection ?? BookmarksSelection())
.trailingDivider()
BookmarkShareCommands(selection: selection)
BookmarkShareCommands(selection: selection ?? BookmarksSelection())
.trailingDivider()
BookmarkTagCommands(selection: selection)
BookmarkTagCommands(selection: selection ?? BookmarksSelection())
}
AccountCommands()
}
Expand Down
38 changes: 0 additions & 38 deletions macos/Bookmarks/Commands/BookmarkDebugCommands.swift

This file was deleted.

3 changes: 2 additions & 1 deletion macos/Bookmarks/Commands/BookmarkDestructiveCommands.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import BookmarksCore
struct BookmarkDesctructiveCommands: View {

@Environment(\.manager) var manager: BookmarksManager
@Environment(\.menuType) var menuType

@ObservedObject var selection: BookmarksSelection

Expand All @@ -33,7 +34,7 @@ struct BookmarkDesctructiveCommands: View {
selection.delete(manager: manager)
}
.contextAwareKeyboardShortcut(.delete, modifiers: [.command])
.mainMenuItemCondition(.nonEmpty, selection)
.disabled(menuType == .main && selection.isEmpty)
}

}
7 changes: 4 additions & 3 deletions macos/Bookmarks/Commands/BookmarkEditCommands.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import BookmarksCore
struct BookmarkEditCommands: View {

@Environment(\.manager) var manager: BookmarksManager
@Environment(\.menuType) var menuType

@ObservedObject var selection: BookmarksSelection

Expand All @@ -33,15 +34,15 @@ struct BookmarkEditCommands: View {
selection.update(manager: manager, toRead: !selection.containsUnreadBookmark)
}
.contextAwareKeyboardShortcut("U", modifiers: [.command, .shift])
.mainMenuItemCondition(.nonEmpty, selection)
.disabled(menuType == .main && selection.isEmpty)
Button(selection.containsPublicBookmark ? "Make Private" : "Make Public") {
selection.update(manager: manager, shared: !selection.containsPublicBookmark)
}
.mainMenuItemCondition(.nonEmpty, selection)
.disabled(menuType == .main && selection.isEmpty)
Divider()
Button("Edit on Pinboard") {
selection.open(manager: manager, location: .pinboard)
}
.mainMenuItemCondition(.nonEmpty, selection)
.disabled(menuType == .main && selection.isEmpty)
}
}
4 changes: 2 additions & 2 deletions macos/Bookmarks/Commands/BookmarkOpenCommands.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ struct BookmarkOpenCommands: View {
selection.open(manager: manager)
}
.contextAwareKeyboardShortcut(.return, modifiers: [.command])
.mainMenuItemCondition(.nonEmpty, selection)
.disabled(menuType == .main && selection.isEmpty)
Button("Open on Internet Archive") {
selection.open(manager: manager, location: .internetArchive)
}
.contextAwareKeyboardShortcut(.return, modifiers: [.command, .shift])
.mainMenuItemCondition(.nonEmpty, selection)
.disabled(menuType == .main && selection.isEmpty)
}

}
5 changes: 3 additions & 2 deletions macos/Bookmarks/Commands/BookmarkShareCommands.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import BookmarksCore
struct BookmarkShareCommands: View {

@Environment(\.manager) var manager: BookmarksManager
@Environment(\.menuType) var menuType

@ObservedObject var selection: BookmarksSelection

Expand All @@ -33,12 +34,12 @@ struct BookmarkShareCommands: View {
selection.copy()
}
.contextAwareKeyboardShortcut("c", modifiers: [.command])
.mainMenuItemCondition(.nonEmpty, selection)
.disabled(menuType == .main && selection.isEmpty)
Button("Copy Tags") {
selection.copyTags()
}
.contextAwareKeyboardShortcut("c", modifiers: [.command, .shift])
.mainMenuItemCondition(.nonEmpty, selection)
.disabled(menuType == .main && selection.isEmpty)
}

}
19 changes: 0 additions & 19 deletions macos/Bookmarks/Model/BookmarksSelection.swift
Original file line number Diff line number Diff line change
Expand Up @@ -108,22 +108,3 @@ extension BookmarksSelection.SheetType: Identifiable {
}

}

public struct SelectionEnvironmentKey: EnvironmentKey {

public static var defaultValue = BookmarksSelection()

}

public extension EnvironmentValues {

var selection: BookmarksSelection {
get { self[SelectionEnvironmentKey.self] }
set { self[SelectionEnvironmentKey.self] = newValue }
}

}

extension BookmarksSelection: Countable {

}
73 changes: 0 additions & 73 deletions macos/Bookmarks/Modifiers/MainMenuItemCondition.swift

This file was deleted.

12 changes: 3 additions & 9 deletions macos/Bookmarks/Views/ContentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,8 @@ struct ContentView: View {

let manager: BookmarksManager

@ObservedObject var selection: BookmarksSelection

@StateObject var bookmarksView: BookmarksView
@StateObject var selection = BookmarksSelection()
@StateObject var selectionTracker: SelectionTracker<Bookmark>
@State var firstResponder: Bool = false

Expand All @@ -39,11 +38,9 @@ struct ContentView: View {

private var subscription: AnyCancellable?

init(selection: BookmarksSelection,
manager: BookmarksManager,
init(manager: BookmarksManager,
section: BookmarksSection) {
self.manager = manager
self.selection = selection
let bookmarksView = Deferred(BookmarksView(manager: manager, section: section))
let selectionTracker = Deferred(SelectionTracker(items: bookmarksView.get().$bookmarks))
_bookmarksView = StateObject(wrappedValue: bookmarksView.get())
Expand All @@ -70,10 +67,6 @@ struct ContentView: View {
BookmarkShareCommands(selection: selection)
.trailingDivider()
BookmarkTagCommands(selection: selection)
#if DEBUG
BookmarkDebugCommands()
.leadingDivider()
#endif
} onContextMenuChange: { focused in
guard focused == true else {
return
Expand Down Expand Up @@ -127,5 +120,6 @@ struct ContentView: View {
}
.navigationTitle(bookmarksView.title)
.navigationSubtitle(bookmarksView.subtitle)
.focusedSceneObject(selection)
}
}
7 changes: 6 additions & 1 deletion macos/Bookmarks/Views/EditView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ struct EditView: View {
}

@Environment(\.manager) var manager
@Environment(\.selection) var selection
@Environment(\.presentationMode) var presentationMode

@FocusedObject var selection: BookmarksSelection?

var bookmarks: [Bookmark]
@State var isBusy = false
@AppStorage(SettingsKey.addTagsMarkAsRead.rawValue) var markAsRead: Bool = false
Expand Down Expand Up @@ -89,6 +90,10 @@ struct EditView: View {
.frame(minWidth: LayoutMetrics.minimumButtonWidth, maxWidth: .infinity)
.keyboardShortcut(.cancelAction)
Button {
guard let selection else {
print("No selection!")
return
}
isBusy = true
let tags = tokens.compactMap { $0.associatedValue }
let updatedBookmarks = bookmarks.map { item in
Expand Down
Loading

0 comments on commit ff922ca

Please sign in to comment.