Skip to content

feat: make drag-and-drop trigger area configurable & fix shelf disable behavior #1056

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tornado-bunk wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

feat: make drag-and-drop trigger area configurable & fix shelf disable behavior #1056

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b872ae3
Make drag-and-drop trigger area configurable
tornado-bunk Feb 25, 2026
2f0772b
fix: properly disable drag-and-drop detection when Shelf is disabled
tornado-bunk Feb 25, 2026
10eac9a
style: fix indentation
tornado-bunk Feb 25, 2026
df5b3be
Update boringNotch/Localizable.xcstrings
Alexander5015 Feb 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions boringNotch/ContentView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ struct ContentView: View {
@State private var isHovering: Bool = false
@State private var anyDropDebounceTask: Task<Void, Never>?

@Default(.boringShelf) var boringShelf
@Default(.expandedDragDetection) var expandedDragDetection

@State private var gestureProgress: CGFloat = .zero

@State private var haptics: Bool = false
Expand Down Expand Up @@ -394,7 +397,22 @@ struct ContentView: View {
.opacity(gestureProgress != 0 ? 1.0 - min(abs(gestureProgress) * 0.1, 0.3) : 1.0)
}
}
.onDrop(of: [.fileURL, .url, .utf8PlainText, .plainText, .data], delegate: GeneralDropTargetDelegate(isTargeted: $vm.generalDropTargeting))
// Disable general drop target if shelf or detection is disabled
.onDrop(of: [.fileURL, .url, .utf8PlainText, .plainText, .data], delegate: GeneralDropTargetDelegate(isTargeted: Binding<Bool>(
get: {
// Return true only if BOTH shelf AND detection are enabled
return boringShelf && expandedDragDetection && vm.generalDropTargeting
},
set: { newValue in
// Only update the actual model IF shelf AND detection are enabled
if boringShelf && expandedDragDetection {
vm.generalDropTargeting = newValue
} else {
// If disabled, force target to false to be safe
vm.generalDropTargeting = false
}
}
)))
}

@ViewBuilder
Expand Down Expand Up @@ -521,7 +539,7 @@ struct ContentView: View {

@ViewBuilder
var dragDetector: some View {
if Defaults[.boringShelf] && vm.notchState == .closed {
if boringShelf && expandedDragDetection && vm.notchState == .closed {
Color.clear
.frame(maxWidth: .infinity, maxHeight: .infinity)
.contentShape(Rectangle())
Expand Down
11 changes: 10 additions & 1 deletion boringNotch/Localizable.xcstrings
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6212,6 +6212,10 @@
}
}
},
"Detection area" : {
"comment" : "A label for the picker that lets the user select the detection area for drag-and-drop functionality.",
"isCommentAutoGenerated" : true
},
"Disable" : {
"extractionState" : "stale",
"localizations" : {
Expand Down Expand Up @@ -7215,6 +7219,10 @@
}
}
},
"Enable drag detection" : {
"comment" : "A toggle that enables or disables drag detection in the shelf.",
"isCommentAutoGenerated" : true
},
"Enable gestures" : {
"localizations" : {
"ar" : {
Expand Down Expand Up @@ -7817,6 +7825,7 @@
}
},
"Expanded drag detection area" : {
"extractionState" : "stale",
"localizations" : {
"ar" : {
"stringUnit" : {
Expand Down Expand Up @@ -20936,5 +20945,5 @@
}
}
},
"version" : "1.0"
"version" : "1.1"
}
32 changes: 25 additions & 7 deletions boringNotch/boringNotchApp.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ class AppDelegate: NSObject, NSApplicationDelegate {
private func setupDragDetectors() {
cleanupDragDetectors()

guard Defaults[.expandedDragDetection] else { return }
guard Defaults[.boringShelf] && Defaults[.expandedDragDetection] else { return }

if Defaults[.showOnAllDisplays] {
for screen in NSScreen.screens {
Expand All @@ -205,19 +205,29 @@ class AppDelegate: NSObject, NSApplicationDelegate {

private func setupDragDetectorForScreen(_ screen: NSScreen) {
Copy link
Member

@Alexander5015 Alexander5015 Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't the existing drop target already cover the closed notch? In this case a global monitor shouldn't be necessary. In general i would like to avoid the global monitor, since it false positives with Safari tabs and is probably less performant than a drop target.

guard let uuid = screen.displayUUID else { return }

let screenFrame = screen.frame
let notchHeight = openNotchSize.height
let notchWidth = openNotchSize.width

// Create notch region at the top-center of the screen where an open notch would occupy
let closedNotchSize = getClosedNotchSize(screenUUID: uuid)

var notchHeight: CGFloat
var notchWidth: CGFloat

switch Defaults[.dragDetectionArea] {
case .openNotch:
notchHeight = openNotchSize.height
notchWidth = openNotchSize.width
case .closedNotch:
notchHeight = closedNotchSize.height
notchWidth = closedNotchSize.width
}

let notchRegion = CGRect(
x: screenFrame.midX - notchWidth / 2,
y: screenFrame.maxY - notchHeight,
width: notchWidth,
height: notchHeight
)

let detector = DragDetector(notchRegion: notchRegion)

detector.onDragEntersNotchRegion = { [weak self] in
Expand Down Expand Up @@ -345,6 +355,14 @@ class AppDelegate: NSObject, NSApplicationDelegate {
}
})

observers.append(NotificationCenter.default.addObserver(
forName: Notification.Name.boringShelfChanged, object: nil, queue: nil
) { [weak self] _ in
Task { @MainActor in
self?.setupDragDetectors()
}
})

// Use closure-based observers for DistributedNotificationCenter and keep tokens for removal
screenLockedObserver = DistributedNotificationCenter.default().addObserver(
forName: NSNotification.Name(rawValue: "com.apple.screenIsLocked"),
Expand Down
22 changes: 21 additions & 1 deletion boringNotch/components/Settings/Views/ShelfSettingsView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ import SwiftUI

struct Shelf: View {

@Default(.boringShelf) var boringShelf: Bool
@Default(.shelfTapToOpen) var shelfTapToOpen: Bool
@Default(.quickShareProvider) var quickShareProvider
@Default(.expandedDragDetection) var expandedDragDetection: Bool
@Default(.dragDetectionArea) var dragDetectionArea: DragDetectionArea
@StateObject private var quickShareService = QuickShareService.shared

private var selectedProvider: QuickShareProvider? {
Expand All @@ -25,18 +27,36 @@ struct Shelf: View {
Defaults.Toggle(key: .boringShelf) {
Text("Enable shelf")
}
.onChange(of: boringShelf) {
NotificationCenter.default.post(
name: Notification.Name.boringShelfChanged,
object: nil
)
}
Defaults.Toggle(key: .openShelfByDefault) {
Text("Open shelf by default if items are present")
}
Defaults.Toggle(key: .expandedDragDetection) {
Text("Expanded drag detection area")
Text("Enable drag detection")
Copy link
Member

@Alexander5015 Alexander5015 Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating a new setting while still using an older variable is generally bad practice. In the current version, this value only disabled drag detection outside of the closed notch window, but now it is being used to disable all drag detection. If this code gets deployed, it will apply the old setting value to the new unrelated setting, causing unexpected behaviour for users by disabling dragging entirely.

}
.onChange(of: expandedDragDetection) {
NotificationCenter.default.post(
name: Notification.Name.expandedDragDetectionChanged,
object: nil
)
}
Picker("Detection area", selection: $dragDetectionArea) {
ForEach(DragDetectionArea.allCases) { area in
Text(area.rawValue).tag(area)
}
}
.disabled(!expandedDragDetection)
.onChange(of: dragDetectionArea) {
NotificationCenter.default.post(
Copy link
Member

@Alexander5015 Alexander5015 Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we eventually need to move away from using notifications for these settings, I’m not requesting a change here since this pattern is still consistent.

Copy link
Member

@theboringhumane theboringhumane Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for now this is good.

name: Notification.Name.expandedDragDetectionChanged,
object: nil
)
}
Comment on lines +48 to +59
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The UI presents "Enable drag detection" toggle and a "Detection area" picker, but the implementation creates a confusing relationship. The picker is disabled when drag detection is off, but the underlying dragDetectionArea setting still exists and is used even when expandedDragDetection is false (though setupDragDetectors early-returns). This creates semantic confusion: what does it mean to have a "detection area" setting when detection is disabled? Consider one of these approaches: (1) Hide the picker entirely when detection is disabled instead of just disabling it, or (2) Rename the toggle to better reflect that it enables/disables the global monitoring system, while the picker controls the active area when enabled.

Suggested change
Picker("Detection area", selection: $dragDetectionArea) {
ForEach(DragDetectionArea.allCases) { area in
Text(area.rawValue).tag(area)
}
}
.disabled(!expandedDragDetection)
.onChange(of: dragDetectionArea) {
NotificationCenter.default.post(
name: Notification.Name.expandedDragDetectionChanged,
object: nil
)
}
if expandedDragDetection {
Picker("Detection area", selection: $dragDetectionArea) {
ForEach(DragDetectionArea.allCases) { area in
Text(area.rawValue).tag(area)
}
}
.onChange(of: dragDetectionArea) {
NotificationCenter.default.post(
name: Notification.Name.expandedDragDetectionChanged,
object: nil
)
}
}

Copilot uses AI. Check for mistakes.
Copy link
Author

@tornado-bunk tornado-bunk Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When expandedDragDetection is false we early‑return from setupDragDetectors(), so dragDetectionArea isn’t used at runtime; the disabled picker only shows (and preserves) which area will be used once detection is turned back on.

Defaults.Toggle(key: .copyOnDrag) {
Text("Copy items on drag")
}
Expand Down
11 changes: 10 additions & 1 deletion boringNotch/models/Constants.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ extension Notification.Name {

// MARK: - Shelf
static let expandedDragDetectionChanged = Notification.Name("expandedDragDetectionChanged")
static let boringShelfChanged = Notification.Name("boringShelfChanged")

// MARK: - System
static let accessibilityAuthorizationChanged = Notification.Name("accessibilityAuthorizationChanged")
Expand Down Expand Up @@ -72,6 +73,13 @@ enum SneakPeekStyle: String, CaseIterable, Identifiable, Defaults.Serializable {
var id: String { self.rawValue }
}

enum DragDetectionArea: String, CaseIterable, Identifiable, Defaults.Serializable {
case openNotch = "Expanded area"
case closedNotch = "Notch only"

var id: String { self.rawValue }
}

// Action to perform when Option (⌥) is held while pressing media keys
enum OptionKeyAction: String, CaseIterable, Identifiable, Defaults.Serializable {
case openSettings = "Open System Settings"
Expand Down Expand Up @@ -191,7 +199,8 @@ extension Defaults.Keys {
static let copyOnDrag = Key<Bool>("copyOnDrag", default: false)
static let autoRemoveShelfItems = Key<Bool>("autoRemoveShelfItems", default: false)
static let expandedDragDetection = Key<Bool>("expandedDragDetection", default: true)

static let dragDetectionArea = Key<DragDetectionArea>("dragDetectionArea", default: .openNotch)

// MARK: Calendar
static let calendarSelectionState = Key<CalendarSelectionState>("calendarSelectionState", default: .all)
static let hideAllDayEvents = Key<Bool>("hideAllDayEvents", default: false)
Expand Down
4 changes: 2 additions & 2 deletions boringNotch/observers/DragDetector.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ final class DragDetector {
.string
]
let isValid = dragPasteboard.pasteboardItems?.allSatisfy { item in
item.types.allSatisfy { validTypes.contains($0) }
item.types.contains { validTypes.contains($0) }
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change from allSatisfy to contains changes the validation logic in a way that weakens content validation. Previously, the code verified that ALL types in each pasteboard item were valid types. Now it only checks if ANY type in each item is valid. This means items with a mix of valid and invalid types would now pass validation when they would have been rejected before. Consider whether this relaxed validation is intentional. If the goal is to accept items that have at least one valid type (regardless of other types), this is correct. Otherwise, revert to allSatisfy.

Suggested change
item.types.contains { validTypes.contains($0) }
item.types.allSatisfy { validTypes.contains($0) }

Copilot uses AI. Check for mistakes.
Copy link
Author

@tornado-bunk tornado-bunk Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, changing from allSatisfy to contains is intentional.
Pasteboard items (e.g. from Finder) usually include many extra types (dyn., com.apple.finder., etc.) in addition to .fileURL / URL / string.
With allSatisfy, a single non-whitelisted type caused us to reject otherwise valid drags, which is why drag detection stopped working reliably after the first drag during my tests.

}
return isValid ?? false
}
Expand Down Expand Up @@ -79,7 +79,7 @@ final class DragDetector {
if self.isContentDragging {
let mouseLocation = NSEvent.mouseLocation
self.onDragMove?(mouseLocation)

// Track notch region entry/exit
let containsMouse = self.notchRegion.contains(mouseLocation)
if containsMouse && !self.hasEnteredNotchRegion {
Expand Down
Loading