diff --git a/DankRssWidget.qml b/DankRssWidget.qml index 19759da..137354b 100644 --- a/DankRssWidget.qml +++ b/DankRssWidget.qml @@ -97,10 +97,16 @@ DesktopPluginComponent { onTriggered: root._overviewGuard = false } - // D8: only steal keyboard focus while the search field is actually - // revealed, so the widget never intercepts keys (e.g. compositor - // keybinds) meant for other DMS surfaces while search is closed. - property bool acceptsKeyboardFocus: root.searchActive + // D8: DMS maps this onto WlrKeyboardFocus.OnDemand (surface-eligible, + // not surface-focused) vs. None. Widening it to include pointer hover + // does not reintroduce the keybind-swallowing problem this guard exists + // for: OnDemand never grants focus on its own, it only lets a click that + // lands on us claim it. While the pointer merely rests over the widget + // with nothing clicked, keys still go to niri untouched. This is needed + // because the surface must already be focus-eligible *before* the click + // that opens search, or that very click grants nothing and typing is a + // no-op until a second click (see search-fixes design doc, Problem 1). + property bool acceptsKeyboardFocus: root.searchActive || widgetHover.hovered // Read tracking, keyed by stable item id. `readMap` is replaced (not mutated) // so QML property-change notification fires; `readOrder` keeps newest-first @@ -115,13 +121,27 @@ DesktopPluginComponent { property var bookmarkOrder: [] // Selection is TRANSIENT: never persisted, never bounded/capped like - // readOrder/bookmarkOrder — it is a working set over currently-visible - // items only. Plain map (not an id-order list) because membership is - // all that matters; order is irrelevant. Mirrors readMap/bookmarkMap's - // "map alongside a QML property, replaced not mutated" pattern. + // readOrder/bookmarkOrder. Pruned only against root.allItems (S10), so it + // survives search/filter-chip changes and can include ids currently + // hidden by the active filter -- not just what's on screen. Plain map + // (not an id-order list) because membership is all that matters; order + // is irrelevant. Mirrors readMap/bookmarkMap's "map alongside a QML + // property, replaced not mutated" pattern. property var selectedMap: ({}) readonly property int selectedCount: ReaderState.countSelected(root.selectedMap) + // The visible set applyFilter last built, cached so the selection bar can + // ask "how many selected items are off-screen right now?" without redoing + // the filter pass -- and, more importantly, so that answer changes on the + // same debounced beat as the list itself rather than on every keystroke. + property var visibleItems: [] + + // Shared by searchToggleComponent and both Loaders that instantiate it. + // A Loader given an explicit Layout size does NOT stretch its item to fit, + // so the button only lands correctly while the two numbers agree -- with a + // literal in each place, drift would silently leave dead click area. + readonly property int searchToggleSize: 22 + // Per-feed runtime status, mirrored to the state tier for the settings panel property var feedStatuses: [] @@ -956,9 +976,15 @@ DesktopPluginComponent { }); } - // S10: prune selection against the newly-rebuilt visible set so - // selectedCount can never exceed what's on screen. - root.selectedMap = ReaderState.pruneSelected(root.selectedMap, visible); + // S10: prune selection against the full dataset (root.allItems), not + // the newly-rebuilt visible set -- selection must survive search and + // filter-chip changes and only drop an id once it leaves the dataset + // entirely (e.g. a refresh evicting an old item). selectedCount can + // therefore exceed what's on screen; bulk actions already iterate + // selectedMap rather than the visible model, so this is safe, and the + // selection-bar label below surfaces the hidden portion explicitly. + root.selectedMap = ReaderState.pruneSelected(root.selectedMap, root.allItems); + root.visibleItems = visible; } onFilterModeChanged: root.applyFilter() @@ -976,6 +1002,17 @@ DesktopPluginComponent { border.color: Theme.withAlpha(root.resolvedBorderColor, root.borderOpacity) clip: true + // Backs D8's acceptsKeyboardFocus above. A pointer handler, not a + // MouseArea: the widget is full of child MouseAreas (filterArea, + // markAllArea, per-item areas) and a parent MouseArea's containsMouse + // goes false whenever a hover-enabled child takes the pointer, so the + // flag would flicker exactly while the user aims at the search + // button. HoverHandler observes the pointer over its parent's bounds + // without competing for the event. + HoverHandler { + id: widgetHover + } + ColumnLayout { anchors.fill: parent anchors.margins: Theme.spacingM @@ -1060,6 +1097,32 @@ DesktopPluginComponent { color: Theme.outlineVariant } + // Search toggle, shared by the actions bar and the selection bar + // (S6) so the two copies cannot drift out of sync. Layout.* + // sizing is set on the Loader that instantiates this, not here -- + // a Component's root item isn't a direct RowLayout child, so + // attached properties set inside it are ignored by the layout. + Component { + id: searchToggleComponent + + DankActionButton { + iconName: root.searchActive ? "search_off" : "search" + iconSize: 14 + buttonSize: root.searchToggleSize + iconColor: (root.searchActive || root.searching) ? Theme.primary : Theme.surfaceVariantText + onClicked: { + root.searchActive = !root.searchActive; + // Closing search must not leave an invisible query + // silently filtering the list. + if (!root.searchActive && root.searchQuery !== "") { + searchField.clear(); + root.searchQuery = ""; + root.applyFilter(); + } + } + } + } + // --- Actions bar: filter + mark all (normal mode) --- RowLayout { Layout.fillWidth: true @@ -1113,23 +1176,10 @@ DesktopPluginComponent { // Search toggle. Search gets its own row when revealed so the // filter chips stay readable at narrow widget widths. - DankActionButton { - iconName: root.searchActive ? "search_off" : "search" - iconSize: 14 - buttonSize: 22 - Layout.preferredWidth: 22 - Layout.preferredHeight: 22 - iconColor: (root.searchActive || root.searching) ? Theme.primary : Theme.surfaceVariantText - onClicked: { - root.searchActive = !root.searchActive; - // Closing search must not leave an invisible query - // silently filtering the list. - if (!root.searchActive && root.searchQuery !== "") { - searchField.clear(); - root.searchQuery = ""; - root.applyFilter(); - } - } + Loader { + Layout.preferredWidth: root.searchToggleSize + Layout.preferredHeight: root.searchToggleSize + sourceComponent: searchToggleComponent } // Mark all read / unread toggle @@ -1186,8 +1236,20 @@ DesktopPluginComponent { spacing: Theme.spacingXS visible: root.selectedCount > 0 + // S10: selection can now include ids hidden by the active + // filter/search (pruned only against root.allItems), so the + // label must say so rather than silently undercounting what + // "N selected" implies is on screen. Derived from + // root.visibleItems (the set applyFilter last built) rather + // than re-running filterItems here: that would both duplicate + // the scan and read root.searchQuery live, so the count would + // race ahead of the list during searchDebounce's 150ms and + // briefly disagree with what is on screen. + readonly property int hiddenSelected: root.selectedCount - ReaderState.countSelectedIn(root.selectedMap, root.visibleItems) + StyledText { text: root.selectedCount + " selected" + + (selectionActionsRow.hiddenSelected > 0 ? " (" + selectionActionsRow.hiddenSelected + " hidden)" : "") font.pixelSize: root.fontSize - 2 color: Theme.surfaceVariantText Layout.fillWidth: true @@ -1266,6 +1328,16 @@ DesktopPluginComponent { } } + // Search toggle (Problem 2): the header's filter/search row + // is replaced by this bar while items are selected, so + // search needs its own entry point here too, sharing the + // header's exact behaviour via searchToggleComponent. + Loader { + Layout.preferredWidth: root.searchToggleSize + Layout.preferredHeight: root.searchToggleSize + sourceComponent: searchToggleComponent + } + // Clear selection — icon-only always (never needs a label; "X" // reads as "clear" without text at any width). DankActionButton { @@ -1302,8 +1374,15 @@ DesktopPluginComponent { } onVisibleChanged: { - if (visible) + // D8/Problem 1: focus arrival is not synchronous with the + // click that revealed us (seat focus grant races Qt's + // internal focus item), so a single forceActiveFocus() + // can land before the surface is actually eligible. The + // deferred retry catches that case. + if (visible) { forceActiveFocus(); + Qt.callLater(forceActiveFocus); + } } } diff --git a/ReaderState.js b/ReaderState.js index f2c8c36..a58c11b 100644 --- a/ReaderState.js +++ b/ReaderState.js @@ -388,8 +388,25 @@ function countSelected(selectedMap) { return Object.keys(selectedMap).filter(function (k) { return selectedMap[k]; }).length; } -// Drop any selected id whose item is no longer in `items` (S10) — keeps -// selectedCount from ever exceeding what's currently visible. +// Count how many selected ids are present in `items` (the intersection). +// Used to render the "N hidden" portion of the selection count once +// selection is allowed to exceed the visible/filtered set. +function countSelectedIn(selectedMap, items) { + if (!selectedMap) return 0; + var present = buildIdMap((items || []).map(function (i) { return i ? i.id : ""; })); + var count = 0; + for (var k in selectedMap) { + if (selectedMap[k] && present[k]) { + count++; + } + } + return count; +} + +// Drop any selected id whose item has left the dataset entirely (S10). The +// caller passes the full dataset, not the filtered/visible view — selection +// is intentionally allowed to exceed what's on screen (search, filter chips) +// and is only cleared for an id when a refresh evicts it from `items`. function pruneSelected(selectedMap, items) { if (!selectedMap) return {}; var present = buildIdMap((items || []).map(function (i) { return i ? i.id : ""; })); @@ -532,6 +549,7 @@ if (typeof module !== "undefined" && module.exports) { toggleSelected: toggleSelected, clearSelection: clearSelection, countSelected: countSelected, + countSelectedIn: countSelectedIn, pruneSelected: pruneSelected, addAllBookmarked: addAllBookmarked, reconcileServerStatus: reconcileServerStatus diff --git a/tests/reader-state.test.js b/tests/reader-state.test.js index 28de32d..203384d 100644 --- a/tests/reader-state.test.js +++ b/tests/reader-state.test.js @@ -520,6 +520,54 @@ describe("selection", () => { assert.deepStrictEqual(R.pruneSelected({}, [{ id: "a" }]), {}); assert.deepStrictEqual(R.pruneSelected(null, [{ id: "a" }]), {}); }); + + test("pruneSelected keeps an id present in the dataset but absent from a filtered view", () => { + const allItems = [{ id: "a" }, { id: "b" }, { id: "c" }]; + const filteredView = [{ id: "b" }]; + const out = R.pruneSelected({ a: true, b: true, c: true }, allItems); + assert.deepStrictEqual(out, { a: true, b: true, c: true }); + // Sanity check: the same ids would have been dropped against the filtered view. + assert.deepStrictEqual(R.pruneSelected({ a: true, b: true, c: true }, filteredView), { b: true }); + }); + + test("pruneSelected still drops an id absent from the dataset entirely", () => { + const allItems = [{ id: "a" }, { id: "c" }]; + const out = R.pruneSelected({ a: true, b: true, c: true }, allItems); + assert.deepStrictEqual(out, { a: true, c: true }); + }); + + test("countSelectedIn returns the size of the intersection", () => { + const items = [{ id: "a" }, { id: "b" }, { id: "c" }]; + assert.strictEqual(R.countSelectedIn({ a: true, b: true }, items), 2); + assert.strictEqual(R.countSelectedIn({ a: true, z: true }, items), 1); + }); + + test("countSelectedIn returns 0 for an empty or null map", () => { + const items = [{ id: "a" }]; + assert.strictEqual(R.countSelectedIn({}, items), 0); + assert.strictEqual(R.countSelectedIn(null, items), 0); + }); + + test("countSelectedIn ignores keys whose value is falsy", () => { + const items = [{ id: "a" }, { id: "b" }]; + assert.strictEqual(R.countSelectedIn({ a: true, b: false }, items), 1); + }); + + test("selection survives a filter round-trip: select, filter down, restore", () => { + const allItems = [{ id: "a" }, { id: "b" }, { id: "c" }]; + let selectedMap = {}; + selectedMap = R.toggleSelected(selectedMap, "a"); + selectedMap = R.toggleSelected(selectedMap, "b"); + selectedMap = R.toggleSelected(selectedMap, "c"); + + const filteredView = [{ id: "b" }]; + selectedMap = R.pruneSelected(selectedMap, allItems); + assert.strictEqual(R.countSelectedIn(selectedMap, filteredView), 1); + assert.strictEqual(R.countSelected(selectedMap), 3); + + selectedMap = R.pruneSelected(selectedMap, allItems); + assert.strictEqual(R.countSelected(selectedMap), 3); + }); }); describe("addAllBookmarked", () => {