diff --git a/Panel.qml b/Panel.qml index 7306b77..a321e7c 100644 --- a/Panel.qml +++ b/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,7 @@ Panel { function openUrl(url) { var value = String(url || "") if (value === "") return - Quickshell.execDetached(["omarchy-launch-browser", value]) + Quickshell.execDetached(["omarchy-launch-webapp", value]) close() } @@ -232,6 +266,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 +451,7 @@ Panel { contentHeight: height clip: true flickableDirection: Flickable.HorizontalFlick + interactive: contentWidth > width Row { id: filterRow spacing: Style.space(6) @@ -752,7 +797,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 +844,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 diff --git a/README.md b/README.md index 9036874..8452434 100644 --- a/README.md +++ b/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 in a web app window; 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 | @@ -112,6 +112,8 @@ omarchy plugin remove robzolkos.github | `Escape` in search | Clear search and return to row navigation | | `Escape` elsewhere | Close the panel | +Rows open through `omarchy-launch-webapp`, so GitHub gets a dedicated app window rather than a tab in an already-crowded browser. That helper targets Chromium-based default browsers and falls back to `chromium.desktop`, so a machine without any Chromium-based browser installed will not open links. + Activity sections show five items initially and expand to a bounded list of 25. **Open in GitHub** takes you to the corresponding complete GitHub view where one is available. The notifications footer also carries **Mark all read**. The first click captures the displayed notification snapshot and changes the label to **Confirm?**; only the second click sends the request. The confirmation lapses after a few seconds, when the panel closes, when a refresh changes the notification list, and whenever another mark is running. Notifications before the newest displayed second are handled in bulk, while displayed threads from that boundary second are marked by ID so same-second arrivals stay unread. The dashboard refreshes from GitHub after every attempt; large inboxes processed asynchronously may briefly retain threads that are already on their way out. diff --git a/Service.qml b/Service.qml index 1bb8985..ace97b5 100644 --- a/Service.qml +++ b/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. @@ -104,15 +109,115 @@ Item { } function helperPath() { - return Qt.resolvedUrl("omarchy-github-fetch").toString().replace(/^file:\/\//, ""); + return decodeURIComponent(Qt.resolvedUrl("omarchy-github-fetch").toString().replace(/^file:\/\//, "")); } function command() { 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; diff --git a/tests/panel-source-test.sh b/tests/panel-source-test.sh index 6ea765f..df09cc0 100755 --- a/tests/panel-source-test.sh +++ b/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(["omarchy-launch-webapp", value])' \ + "links do not open in a web app window" +assert_not_contains '["omarchy-launch-browser"' \ + "links still open in a browser tab" +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" diff --git a/tests/service-source-test.sh b/tests/service-source-test.sh index b76d14f..5c45d35 100755 --- a/tests/service-source-test.sh +++ b/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 : "";' \