Skip to content

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

Open
tornado-bunk wants to merge 4 commits intoTheBoredTeam:devfrom
tornado-bunk:feat/drag-detection-area-config
Open

feat: make drag-and-drop trigger area configurable & fix shelf disable behavior#1056
tornado-bunk wants to merge 4 commits intoTheBoredTeam:devfrom
tornado-bunk:feat/drag-detection-area-config

Conversation

@tornado-bunk
Copy link

This PR addresses some issues regarding the Shelf's drag-and-drop functionality.

First, it replaces the binary "Expanded drag detection" toggle with a more flexible DragDetectionArea configuration, allowing users to choose between the expanded area (full open notch size) or restricting detection strictly to the notch only.

Second, it fixes a bug where the Shelf would continue to detect drag events even when the "Enable shelf" setting was disabled. Previously, the global drag detectors and the onDrop modified in ContentView did not respect the disabled state of the shelf.

Changes

  • New Feature: Added DragDetectionArea enum and picker in Settings -> Shelf. Options: "Expanded area" (default) and "Notch only".
  • Bug Fix: Updated setupDragDetectors() in boringNotchApp.swift to check Defaults[.boringShelf] before starting monitors.
  • Bug Fix: Modified ContentView.swift's dragDetector and general onDrop delegate to be conditional based on both boringShelf and expandedDragDetection settings.
  • Refactor: Added boringShelfChanged notification to reactively update drag detector state without app restart.

Test Plan

  1. Configuration Test:

    • Go to Settings -> Shelf.
    • Set "Detection area" to "Notch only".
    • Drag a file to the physical notch -> Shelf should open.
    • Drag a file near the notch corners (outside physical notch) -> Shelf should NOT open.
  2. Disable Shelf Test:

    • Toggle "Enable shelf" OFF.
    • Drag a file to the notch -> Shelf should NOT open.
  3. Disable Drag Detection Test:

    • Toggle "Enable shelf" ON, "Enable drag detection" OFF.
    • Drag a file to the notch -> Shelf should NOT open.

Screenshots

Screenshot 2026-02-25 alle 20 43 22

Screen Recordings

Registrazione.schermo.2026-02-25.alle.20.45.57.mp4

tornado-bunk and others added 2 commits February 25, 2026 11:43
Introduce a user-facing setting to choose between an expanded and notch-only drag detection region and use the selected value when computing the shelf drag hotspot. Also fix a pre-existing bug in drag content validation so drag detection continues to work reliably across multiple drags.

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit resolves a bug where dragging files onto the notch (or the expanded detection area) would still trigger the Shelf to open even when the "Enable Shelf" setting was disabled.

The issue was caused by two main factors:
1. The global drag detectors in `boringNotchApp` (which handle the physical notch area and expanded zone) were only checking the `expandedDragDetection` setting, ignoring the global `boringShelf` toggle.
2. The `onDrop` modifier in `ContentView` (which handles drops directly onto the closed notch UI) was unconditionally active, meaning any drag event over the window would trigger the shelf logic regardless of user preferences.

This fix implements a robust solution by:
- **Updating Drag Detectors**: `setupDragDetectors()` now strictly checks that both `boringShelf` and `expandedDragDetection` are enabled before activating any monitors.
- **Introducing New Observers**: Added a `boringShelfChanged` notification that fires when the Shelf is toggled in settings, ensuring drag detectors are immediately started or stopped without requiring an app restart.
- **Conditional Drop Targets**: Modified the `dragDetector` view and the general `onDrop` delegate in `ContentView` to respect the `boringShelf` and `expandedDragDetection` states. The general drop delegate now actively prevents the drop target state from becoming true if the feature is disabled.
Copilot AI review requested due to automatic review settings February 25, 2026 19:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the Shelf's drag-and-drop functionality by making the detection area configurable and fixing a bug where drag detection persisted when the shelf was disabled.

Changes:

  • Replaced binary "Expanded drag detection" toggle with a more flexible system: a toggle for "Enable drag detection" and a picker for "Detection area" (Expanded area vs. Notch only)
  • Fixed bug where drag detectors were active even when "Enable shelf" was disabled by adding boringShelf check to setupDragDetectors() and ContentView drop handlers
  • Added boringShelfChanged notification to reactively update drag detector state

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
boringNotch/observers/DragDetector.swift Modified drag content validation logic and removed trailing whitespace
boringNotch/models/Constants.swift Added DragDetectionArea enum, boringShelfChanged notification, and dragDetectionArea default setting
boringNotch/components/Settings/Views/ShelfSettingsView.swift Added detection area picker, renamed toggle, and added notification handlers for shelf enable/disable
boringNotch/boringNotchApp.swift Updated setupDragDetectors() to respect both boringShelf and expandedDragDetection settings, added support for configurable detection area sizes, and added boringShelfChanged notification observer
boringNotch/Localizable.xcstrings Added localization entries for new UI strings and marked old string as stale
boringNotch/ContentView.swift Added conditional logic to disable drop delegates when shelf or drag detection is disabled

]
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

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.

Comment on lines +48 to +59
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
)
}
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

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.

@@ -205,19 +205,29 @@ class AppDelegate: NSObject, NSApplicationDelegate {

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

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.

}
Defaults.Toggle(key: .expandedDragDetection) {
Text("Expanded drag detection area")
Text("Enable drag detection")
Copy link
Member

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.

}
.disabled(!expandedDragDetection)
.onChange(of: dragDetectionArea) {
NotificationCenter.default.post(
Copy link
Member

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants