Open notifications on click, faster wheel scroll, and xdg-open for links

Panel.qml

@@ -27,6 +27,10 @@ Panel {
   property bool issuesExpanded: false
   property bool actionsExpanded: false
   property bool failuresExpanded: false
+  // Carry sub-notch wheel deltas between events. Touchpads emit many small
+  // angleDeltas; mice often emit a fake 1–2px pixelDelta that would otherwise
+  // crawl the dashboard a couple of pixels per click.
+  property real wheelAccumulator: 0
   readonly property int activityPreviewCount: 5
   readonly property int activityExpandedCount: 25
   readonly property var metricFilters: [
@@ -78,12 +82,42 @@ Panel {
   }
   function activateCursor() {
     if (!selectedTarget) return
-    openUrl(selectedTarget.row.url)
+    openRow(selectedTarget.kind, selectedTarget.row.id, selectedTarget.row.url)
+  }
+  // Snapshot id/url before marking. hideNotification destroys the row, and
+  // reading linkRow.url after that leaves openUrl with an empty target.
+  function openRow(kind, id, url) {
+    var target = String(url || "")
+    var notificationId = String(id || "")
+    openUrl(target)
+    if (kind === "notification") github.markNotificationRead(notificationId)
   }
   function markSelectedRead() {
-    if (github.loading || github.marking) return
     if (selectedTarget && selectedTarget.kind === "notification") github.markNotificationRead(String(selectedTarget.row.id || ""))
   }
+  function applyPanelWheel(event) {
+    if (!panelFlick || (sortPicker && sortPicker.popup.visible)) return false
+    var maxY = Math.max(0, panelFlick.contentHeight - panelFlick.height)
+    if (maxY <= 0) return false
+    var pixel = event.pixelDelta.y
+    var angle = event.angleDelta.y
+    var wheel = Util.wheelSteps(root.wheelAccumulator, angle)
+    root.wheelAccumulator = wheel.remainder
+    // A mouse notch is 120°. Move about one dashboard row per notch.
+    if (wheel.steps !== 0) {
+      panelFlick.contentY = Math.max(0, Math.min(maxY, panelFlick.contentY - wheel.steps * Style.space(80)))
+      return true
+    }
+    // Touchpads report a real pixelDelta larger than Qt's angle conversion.
+    // Scale it so two-finger scroll matches the notch distance above.
+    if (pixel !== 0 && Math.abs(pixel) > Math.abs(angle) / 8) {
+      root.wheelAccumulator = 0
+      panelFlick.contentY = Math.max(0, Math.min(maxY, panelFlick.contentY - pixel * 3))
+      return true
+    }
+    // Swallow leftover high-res angle crumbs so Flickable cannot crawl 1–2px.
+    return angle !== 0 || pixel !== 0
+  }
   function scrollItemIntoView(item) {
     if (!panelFlick || !item) return
     Qt.callLater(function() {
@@ -113,7 +147,9 @@ Panel {
   function openUrl(url) {
     var value = String(url || "")
     if (value === "") return
-    Quickshell.execDetached(["omarchy-launch-browser", value])
+    // Hand the URL to the existing browser so it becomes a tab. omarchy-launch-browser
+    // starts a new uwsm unit per click, which Brave/Chromium often turn into a window.
+    Quickshell.execDetached(["xdg-open", value])
     close()
   }
 
@@ -232,6 +268,16 @@ Panel {
         flickableDirection: Flickable.VerticalFlick
         interactive: contentHeight > height
         ScrollBar.vertical: ScrollBar { policy: ScrollBar.AsNeeded }
+        // Must be a direct child of Flickable or Qt keeps the default
+        // 1–2px wheel distance and this handler never runs.
+        WheelHandler {
+          acceptedDevices: PointerDevice.Mouse | PointerDevice.TouchPad
+          orientation: Qt.Vertical
+          grabPermissions: PointerHandler.CanTakeOverFromAnything
+          onWheel: function(event) {
+            if (root.applyPanelWheel(event)) event.accepted = true
+          }
+        }
 
         Column {
           id: content
@@ -407,6 +453,7 @@ Panel {
             contentHeight: height
             clip: true
             flickableDirection: Flickable.HorizontalFlick
+            interactive: contentWidth > width
             Row {
               id: filterRow
               spacing: Style.space(6)
@@ -752,7 +799,7 @@ Panel {
       hoverEnabled: true
       cursorShape: Qt.PointingHandCursor
       onEntered: root.selectKey(linkRow.cursorKey)
-      onClicked: root.openUrl(linkRow.url)
+      onClicked: root.openRow(linkRow.rowKind, linkRow.notificationId || linkRow.rowId, linkRow.url)
     }
     RowLayout {
       id: row
@@ -799,7 +846,7 @@ Panel {
       }
       PanelActionButton {
         visible: linkRow.showReadAction
-        enabled: !github.loading && !github.marking
+        enabled: github.markingNotificationId !== linkRow.notificationId
         iconText: github.markingNotificationId === linkRow.notificationId ? "󰑐" : "󰄬"
         tooltipText: "Mark this notification read (M)"
         foreground: root.foreground

README.md

@@ -100,8 +100,8 @@ omarchy plugin remove robzolkos.github
 | --- | --- |
 | Left click Octocat | Open or close the dashboard |
 | Right or middle click Octocat | Refresh |
-| Click a row | Open it on GitHub |
-| Check button on a notification | Mark the thread read after GitHub confirms it |
+| Click a row | Open it on GitHub; notification rows are also marked read |
+| Check button on a notification | Mark the thread read immediately, then confirm with GitHub |
 | **Mark all read** in the notifications footer | Arm the bulk mark-as-read |
 | **Confirm?** on the armed button | Mark every notification on screen read |
 | `j` / `k` or arrow keys | Move through visible rows |

Service.qml

@@ -34,6 +34,11 @@ Item {
     property string notificationActionStatus: ""
     property string _markStdout: ""
     property string _markStderr: ""
+    // Thread IDs waiting for PATCH after GitHub confirmed them locally. An
+    // in-flight refresh must not restore these rows, or the bar stays lit until
+    // the next poll even though the user already opened or marked the thread.
+    property var hiddenNotifications: ({})
+    property var markQueue: []
     // Single-thread and bulk marking share one process, so the panel gates every
     // entry point on this rather than on whichever flag a given call happens to
     // set. A caller added later inherits the guard instead of having to know.
@@ -111,8 +116,108 @@ Item {
         return [helperPath(), "--include-archived", boolSetting("includeArchived", false) ? "true" : "false", "--include-forks", boolSetting("includeForks", false) ? "true" : "false", "--repository-scope", repositoryMode(), "--include-archived-reviews", boolSetting("includeArchivedReviewRequests", false) ? "true" : "false", "--include-draft-reviews", boolSetting("includeDraftReviewRequests", false) ? "true" : "false", "--action-scan", actionMode(), "--action-repo-limit", String(intSetting("actionScanRepoLimit", 15, 5, 200)), "--concurrency", String(intSetting("actionScanConcurrency", 6, 1, 12)), "--failed-days", String(intSetting("failedActionDays", 7, 1, 30)), "--failed-limit", String(intSetting("failedActionLimit", 20, 1, 100))];
     }
 
+    function copyMap(value) {
+        var copy = {};
+        var source = value || {};
+        for (var key in source)
+            copy[key] = source[key];
+        return copy;
+    }
+
+    function hideNotification(id) {
+        var value = String(id || "");
+        if (value === "")
+            return ;
+
+        var hidden = copyMap(hiddenNotifications);
+        var next = [];
+        var found = false;
+        for (var i = 0; i < notifications.length; i++) {
+            var item = notifications[i];
+            if (String(item.id || "") === value) {
+                hidden[value] = item;
+                found = true;
+            } else {
+                next.push(item);
+            }
+        }
+        if (!found && hidden[value] === undefined)
+            hidden[value] = {id: value};
+
+        hiddenNotifications = hidden;
+        if (found) {
+            notifications = next;
+            notificationsRevision++;
+        }
+    }
+
+    function restoreHiddenNotification(id) {
+        var value = String(id || "");
+        var item = hiddenNotifications[value];
+        var hidden = copyMap(hiddenNotifications);
+        delete hidden[value];
+        hiddenNotifications = hidden;
+        if (!item)
+            return ;
+
+        for (var i = 0; i < notifications.length; i++) {
+            if (String(notifications[i].id || "") === value)
+                return ;
+        }
+        notifications = [item].concat(notifications);
+        notificationsRevision++;
+    }
+
+    function visibleNotifications(rows) {
+        var incoming = Array.isArray(rows) ? rows : [];
+        var hidden = hiddenNotifications || {};
+        var nextHidden = {};
+        var visible = [];
+        for (var i = 0; i < incoming.length; i++) {
+            var item = incoming[i];
+            var id = String(item.id || "");
+            if (hidden[id])
+                nextHidden[id] = item;
+            else
+                visible.push(item);
+        }
+        hiddenNotifications = nextHidden;
+        return visible;
+    }
+
+    function enqueueMark(id) {
+        var value = String(id || "");
+        if (value === "" || markingNotificationId === value)
+            return ;
+
+        for (var i = 0; i < markQueue.length; i++) {
+            if (markQueue[i] === value)
+                return ;
+        }
+        markQueue = markQueue.concat([value]);
+    }
+
+    function startQueuedMark() {
+        if (fetchProcess.running || markProcess.running || markQueue.length === 0)
+            return false;
+
+        var value = String(markQueue[0] || "");
+        markQueue = markQueue.slice(1);
+        if (value === "")
+            return startQueuedMark();
+
+        actionStatusTimer.stop();
+        markingNotificationId = value;
+        notificationActionStatus = "Marking notification read…";
+        _markStdout = "";
+        _markStderr = "";
+        markProcess.command = [helperPath(), "--mark-notification-read", value];
+        markProcess.running = true;
+        return true;
+    }
+
     function refresh() {
-        if (fetchProcess.running || markProcess.running) {
+        if (fetchProcess.running || markProcess.running || markQueue.length > 0) {
             refreshQueued = true;
             return ;
         }
@@ -132,7 +237,7 @@ Item {
             login = String(data.login || "");
             fetchedRepositoryScope = String(data.repositoryScope || "owned");
             fetchedAt = String(data.fetchedAt || "");
-            notifications = Array.isArray(data.notifications) ? data.notifications : [];
+            notifications = visibleNotifications(data.notifications);
             notificationsRevision++;
             reviewRequests = Array.isArray(data.reviewRequests) ? data.reviewRequests : [];
             assignedIssues = Array.isArray(data.assignedIssues) ? data.assignedIssues : [];
@@ -152,16 +257,15 @@ Item {
 
     function markNotificationRead(id) {
         var value = String(id || "");
-        if (value === "" || loading || fetchProcess.running || markProcess.running)
+        if (value === "")
             return ;
 
-        actionStatusTimer.stop();
-        markingNotificationId = value;
-        notificationActionStatus = "Marking notification read…";
-        _markStdout = "";
-        _markStderr = "";
-        markProcess.command = [helperPath(), "--mark-notification-read", value];
-        markProcess.running = true;
+        // Drop the row before GitHub round-trips. Opening a thread while a
+        // refresh is already running used to no-op, so the icon stayed alarming
+        // until the next poll even after the user had seen the notification.
+        hideNotification(value);
+        enqueueMark(value);
+        startQueuedMark();
     }
 
     function canonicalNotificationTimestamp(value) {
@@ -277,6 +381,9 @@ Item {
                 root.state = "error";
                 root.message = stderr !== "" ? stderr : "GitHub data refresh failed.";
             }
+            if (root.startQueuedMark())
+                return ;
+
             if (root.refreshQueued) {
                 root.refreshQueued = false;
                 Qt.callLater(root.refresh);
@@ -311,15 +418,21 @@ Item {
             } catch (error) {
             }
             var all = root.markingAllNotifications;
+            var markedId = root.markingNotificationId;
             if (exitCode === 0 && response && response.state === "ready") {
                 root.notificationActionStatus = all ? "Notifications marked read. Refreshing…" : "Notification marked read. Refreshing…";
             } else {
                 var fallback = all ? "Could not mark all notifications read." : "Could not mark notification read.";
                 root.notificationActionStatus = response && response.message ? String(response.message) : String(markErrors.text || root._markStderr || fallback).trim();
+                if (!all && markedId !== "")
+                    root.restoreHiddenNotification(markedId);
             }
             root.markingNotificationId = "";
             root.markingAllNotifications = false;
             actionStatusTimer.restart();
+            if (root.startQueuedMark())
+                return ;
+
             // GitHub is authoritative after every attempt. This reconciles
             // successful, failed, and partially completed bulk operations.
             root.refreshQueued = false;

tests/panel-source-test.sh

@@ -8,6 +8,9 @@ fail() { echo "FAIL: $*" >&2; exit 1; }
 assert_contains() {
   [[ $PANEL_SOURCE == *"$1"* ]] || fail "$2"
 }
+assert_not_contains() {
+  [[ $PANEL_SOURCE != *"$1"* ]] || fail "$2"
+}
 
 assert_contains 'glyph: broken ? "󰅖" : (running ? "󰑮" : (checks === "SUCCESS" ? "󰄬" : ""))' \
   "authored pull requests without checks do not use the pull request glyph"
@@ -25,10 +28,27 @@ assert_contains $'onActionBusyChanged: if (section.actionBusy) section.disarmAct
   "bulk confirmation is not invalidated when notification state changes"
 assert_contains $'var confirmed = section.preparedAction\n          section.disarmAction()\n          section.actionTriggered(confirmed)' \
   "bulk action does not submit the originally prepared snapshot"
-assert_contains $'function markSelectedRead() {\n    if (github.loading || github.marking) return' \
-  "keyboard notification marking is enabled during refresh"
-assert_contains $'PanelActionButton {\n        visible: linkRow.showReadAction\n        enabled: !github.loading && !github.marking' \
-  "notification row marking is enabled during refresh"
+assert_contains $'function activateCursor() {\n    if (!selectedTarget) return\n    openRow(selectedTarget.kind, selectedTarget.row.id, selectedTarget.row.url)' \
+  "opening a notification from the keyboard does not mark it read"
+assert_contains $'function openRow(kind, id, url) {\n    var target = String(url || "")\n    var notificationId = String(id || "")\n    openUrl(target)\n    if (kind === "notification") github.markNotificationRead(notificationId)' \
+  "opening a notification marks it before launching the URL"
+assert_contains $'function markSelectedRead() {\n    if (selectedTarget && selectedTarget.kind === "notification") github.markNotificationRead(String(selectedTarget.row.id || ""))' \
+  "keyboard notification marking is blocked during refresh"
+assert_contains $'onClicked: root.openRow(linkRow.rowKind, linkRow.notificationId || linkRow.rowId, linkRow.url)' \
+  "clicking a notification does not open and mark it read"
+assert_contains 'Quickshell.execDetached(["xdg-open", value])' \
+  "links are not handed to the existing browser"
+assert_not_contains '["omarchy-launch-browser"' \
+  "links still start a new uwsm browser unit"
+assert_contains $'PanelActionButton {\n        visible: linkRow.showReadAction\n        enabled: github.markingNotificationId !== linkRow.notificationId' \
+  "notification row marking is disabled during refresh"
+
+assert_contains $'function applyPanelWheel(event) {\n    if (!panelFlick || (sortPicker && sortPicker.popup.visible)) return false' \
+  "the panel still uses Flickable's default wheel distance"
+assert_contains $'panelFlick.contentY = Math.max(0, Math.min(maxY, panelFlick.contentY - wheel.steps * Style.space(80)))' \
+  "a mouse-wheel notch does not move about one row"
+assert_contains $'ScrollBar.vertical: ScrollBar { policy: ScrollBar.AsNeeded }\n        // Must be a direct child of Flickable or Qt keeps the default\n        // 1–2px wheel distance and this handler never runs.\n        WheelHandler {\n          acceptedDevices: PointerDevice.Mouse | PointerDevice.TouchPad' \
+  "the wheel handler is not a direct child of the panel Flickable"
 
 assert_contains 'github.fetchedRepositoryScope === "owned" ? "OWNED REPOSITORIES  " : "REPOSITORIES  "' \
   "the repository heading does not follow the fetched scope"

tests/service-source-test.sh

@@ -21,12 +21,16 @@ assert_not_contains() {
   [[ $SERVICE_SOURCE != *"$1"* ]] || fail "$2"
 }
 
-assert_contains $'function refresh() {\n        if (fetchProcess.running || markProcess.running) {\n            refreshQueued = true;\n            return ;\n        }' \
+assert_contains $'function refresh() {\n        if (fetchProcess.running || markProcess.running || markQueue.length > 0) {\n            refreshQueued = true;\n            return ;\n        }' \
   "refresh and notification marking are not serialized"
-assert_contains $'notifications = Array.isArray(data.notifications) ? data.notifications : [];\n            notificationsRevision++;' \
+assert_contains $'notifications = visibleNotifications(data.notifications);\n            notificationsRevision++;' \
   "notification refreshes do not invalidate prepared confirmations"
-assert_contains $'function markNotificationRead(id) {\n        var value = String(id || "");\n        if (value === "" || loading || fetchProcess.running || markProcess.running)' \
-  "single-notification marking is not blocked during refresh"
+assert_contains $'hideNotification(value);\n        enqueueMark(value);\n        startQueuedMark();' \
+  "single-notification marking is dropped during refresh"
+assert_contains 'notifications = [item].concat(notifications);' \
+  "failed notification marking does not restore the hidden row"
+assert_not_contains $'if (value === "" || loading || fetchProcess.running || markProcess.running)' \
+  "single-notification marking is still blocked during refresh"
 assert_contains $'function canonicalNotificationTimestamp(value) {\n        var text = String(value || "");\n        if (!/^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}Z$/.test(text))\n            return "";' \
   "notification boundaries are not shape validated"
 assert_contains 'return milliseconds <= Date.now() ? text : "";' \