Keep the sidebar's open rows across refreshes
Expanding a workflow run and waiting through one poll closed it again. Two causes. Nova applies whatever collapsible state getTreeItem returns, and it returned Collapsed for every row that was not built default-open, so each reload closed everything the user had opened. On top of that, a reload fired on every poll whether or not anything had changed, so an idle tree was rebuilt every 15 to 60 seconds for no reason. The provider now tracks expansion by node identifier, fed from the tree view's expand and collapse events. A node built default-open seeds that set the first time its identifier is seen, so such a section can still be closed by hand and stay closed. Node identifiers consequently have to be stable across rebuilds. The fallback for nodes without an explicit one was a counter, which changed on every build; it is now derived from the node's kind and name. Step rows were keyed by step number alone, which collided between jobs, and are now qualified by the job. Reloads are also skipped entirely when store.signature() is unchanged. The signature covers everything drawn, including the branch filter, which changes the tree without changing any data. A live run contributes a coarse time bucket so its elapsed clock still ticks, and the explicit Refresh command always redraws. Adds twelve checks covering expansion surviving a rebuild, a default-open section staying closed once closed, and the signature responding to run state and the branch filter but not to a no-op poll. 143 checks total. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01MQuusXgZC2dzwpJJ1qhtti
This commit is contained in:
@@ -29,3 +29,5 @@ Initial release.
|
||||
- Adaptive polling and a notification when a workflow run fails.
|
||||
- Gitea Actions CI running the test suites, script syntax checks, manifest
|
||||
validation, and a generated-image check.
|
||||
- The sidebar keeps its open rows and scroll position across refreshes, and a
|
||||
poll that changes nothing no longer redraws it at all.
|
||||
|
||||
@@ -70,8 +70,12 @@ Controllers mutate through it. It owns:
|
||||
|
||||
- `repos`, `unmatchedHosts`, `instanceStatus`
|
||||
- per-run `jobs`/`artifacts`, per-PR `pullRequestDetail`, `secrets`, `variables`
|
||||
- `onDidChange` → `main.js` debounces 150ms → every provider's `invalidate()` +
|
||||
`reload()`
|
||||
- `onDidChange` → `main.js` debounces 150ms, compares `store.signature()`, and
|
||||
reloads only when it changed. An idle poll therefore leaves the tree, its
|
||||
scroll position, and its open rows completely alone. Anything that changes
|
||||
what is drawn has to appear in that signature — the branch filter is in there
|
||||
for exactly that reason, and a live run contributes a coarse time bucket so
|
||||
its elapsed clock keeps moving.
|
||||
- adaptive polling: `runningInterval` when anything is queued or running, else
|
||||
`idleInterval`. `refresh()` coalesces — a second call while one is in flight
|
||||
returns the same promise rather than stacking.
|
||||
@@ -81,6 +85,15 @@ only `roots()`. Nodes are plain objects (`views/nodes.js` `node()`), turned into
|
||||
`TreeItem`s centrally. A node with `load` fetches when expanded; the result is
|
||||
cached onto `element.children`.
|
||||
|
||||
It also owns **expansion state**, which is not optional bookkeeping. Nova
|
||||
applies whatever `collapsibleState` `getTreeItem` returns, so a reload closes
|
||||
every row the provider does not report as open. `main.js` feeds
|
||||
`onDidExpandElement`/`onDidCollapseElement` into `setExpanded`, keyed by node
|
||||
identifier. A node built with `expanded: true` seeds that set the first time its
|
||||
identifier is seen, so a default-open section can still be closed and stay
|
||||
closed. **Node identifiers must therefore be stable across rebuilds** — never
|
||||
derive one from a counter or from anything that changes between refreshes.
|
||||
|
||||
**`controllers/selection.js`** — a command can arrive from a sidebar (Nova
|
||||
passes the `TreeView`), from a node directly, or from a menu with no argument at
|
||||
all. `selectedNode()` normalises all three; `resolveRepo()` falls back to the
|
||||
|
||||
+44
-3
@@ -52,7 +52,10 @@ function buildSections(store) {
|
||||
return definitions.map((definition) => {
|
||||
const view = new TreeView(definition.id, { dataProvider: definition.provider });
|
||||
nova.subscriptions.add(view);
|
||||
return Object.assign(definition, { view: view });
|
||||
|
||||
const section = Object.assign(definition, { view: view });
|
||||
trackExpansion(section);
|
||||
return section;
|
||||
});
|
||||
}
|
||||
|
||||
@@ -64,7 +67,25 @@ function reloadSections(sections) {
|
||||
}
|
||||
}
|
||||
|
||||
function registerCommands(store, refresh, sections, reviewComments) {
|
||||
/**
|
||||
* Mirrors expansion back into the provider. Nova applies whatever collapsible
|
||||
* state getTreeItem returns, so a reload closes anything the provider does not
|
||||
* know is open.
|
||||
*/
|
||||
function trackExpansion(section) {
|
||||
const expand = section.view.onDidExpandElement((element) =>
|
||||
section.provider.setExpanded(element, true),
|
||||
);
|
||||
const collapse = section.view.onDidCollapseElement((element) =>
|
||||
section.provider.setExpanded(element, false),
|
||||
);
|
||||
|
||||
for (const subscription of [expand, collapse]) {
|
||||
if (subscription) nova.subscriptions.add(subscription);
|
||||
}
|
||||
}
|
||||
|
||||
function registerCommands(store, refresh, reviewComments, redraw) {
|
||||
auth.register(store, refresh);
|
||||
logs.register(store);
|
||||
artifacts.register(store);
|
||||
@@ -75,6 +96,9 @@ function registerCommands(store, refresh, sections, reviewComments) {
|
||||
browser.register();
|
||||
|
||||
nova.commands.register("gitea.refresh", async () => {
|
||||
// An explicit refresh always redraws, even if nothing came back
|
||||
// different, so the command visibly does something.
|
||||
redraw();
|
||||
await refresh({ rediscover: true });
|
||||
await reviewComments.reload({ force: true });
|
||||
});
|
||||
@@ -129,10 +153,27 @@ exports.activate = function () {
|
||||
);
|
||||
|
||||
let timer = null;
|
||||
let lastSignature = null;
|
||||
|
||||
/** Drops the memo so the next change redraws unconditionally. */
|
||||
const redraw = () => {
|
||||
lastSignature = null;
|
||||
};
|
||||
|
||||
const scheduleReload = () => {
|
||||
if (timer) clearTimeout(timer);
|
||||
timer = setTimeout(() => {
|
||||
timer = null;
|
||||
|
||||
// Reloading rebuilds every row, which is disruptive even with
|
||||
// expansion preserved, so a poll that changed nothing does nothing.
|
||||
const signature = store.signature();
|
||||
if (signature === lastSignature) {
|
||||
log.debug("nothing changed; leaving the sidebar alone");
|
||||
return;
|
||||
}
|
||||
lastSignature = signature;
|
||||
|
||||
reloadSections(sections);
|
||||
}, RELOAD_DEBOUNCE_MS);
|
||||
};
|
||||
@@ -144,7 +185,7 @@ exports.activate = function () {
|
||||
log.error("refresh failed", String(error));
|
||||
});
|
||||
|
||||
registerCommands(store, refresh, sections, reviewComments);
|
||||
registerCommands(store, refresh, reviewComments, redraw);
|
||||
watchPreferences(refresh);
|
||||
|
||||
state = {
|
||||
|
||||
@@ -328,6 +328,73 @@ class Store {
|
||||
this.notify();
|
||||
}
|
||||
|
||||
/**
|
||||
* Fingerprint of everything the sidebar draws. `main.js` compares it before
|
||||
* reloading, so an idle poll that changes nothing leaves the tree — and the
|
||||
* user's scroll position and open rows — completely alone.
|
||||
*/
|
||||
signature() {
|
||||
const parts = [];
|
||||
|
||||
// The loading flag only reaches the screen while there is nothing else
|
||||
// to show; including it always would defeat the whole comparison.
|
||||
if (!this.repos.length) parts.push(`loading:${this.loading}`);
|
||||
parts.push(`error:${this.lastError || ""}`);
|
||||
|
||||
for (const entry of this.unmatchedHosts) parts.push(`unmatched:${entry.host}`);
|
||||
|
||||
// The branch filter changes what is drawn without changing any data.
|
||||
const filter = config.branchFilter();
|
||||
parts.push(`filter:${filter.mode}:${filter.branch || ""}`);
|
||||
|
||||
for (const [baseUrl, status] of this.instanceStatus) {
|
||||
parts.push(
|
||||
`instance:${baseUrl}:${status.ok}:${status.version || ""}:${status.user || ""}:${status.error || ""}`,
|
||||
);
|
||||
}
|
||||
|
||||
for (const repo of this.repos) {
|
||||
parts.push(`repo:${repo.key}:${repo.branch || ""}`);
|
||||
|
||||
const runs = this.runsFor(repo.key);
|
||||
parts.push(`runs:${runs.error || ""}:${runs.legacy}`);
|
||||
for (const run of runs.runs) {
|
||||
parts.push(`run:${run.id}:${run.status}:${run.conclusion}:${run.updated_at || ""}`);
|
||||
}
|
||||
|
||||
const pulls = this.pullRequestsFor(repo.key);
|
||||
parts.push(`pulls:${pulls.error || ""}`);
|
||||
for (const pull of pulls.items) {
|
||||
parts.push(`pull:${pull.number}:${pull.state}:${pull.draft}:${pull.updated_at}`);
|
||||
}
|
||||
|
||||
const secrets = this.secrets.get(repo.key);
|
||||
if (secrets) {
|
||||
parts.push(
|
||||
`secrets:${secrets.error || ""}:${secrets.items.map((x) => x.name).join(",")}`,
|
||||
);
|
||||
}
|
||||
|
||||
const variables = this.variables.get(repo.key);
|
||||
if (variables) {
|
||||
parts.push(
|
||||
`variables:${variables.error || ""}:${variables.items
|
||||
.map((x) => `${x.name}=${x.data || x.value || ""}`)
|
||||
.join(",")}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// While something is running its elapsed time is on screen and has to
|
||||
// keep moving, so let the signature change once per active interval.
|
||||
if (this.hasActiveRuns) {
|
||||
const seconds = Math.max(5, config.runningInterval());
|
||||
parts.push(`tick:${Math.floor(Date.now() / 1000 / seconds)}`);
|
||||
}
|
||||
|
||||
return parts.join("|");
|
||||
}
|
||||
|
||||
// -- Housekeeping ------------------------------------------------------
|
||||
|
||||
_pruneCaches() {
|
||||
|
||||
@@ -10,10 +10,10 @@ const { isActiveRun } = require("../store.js");
|
||||
|
||||
const { node, message } = nodes;
|
||||
|
||||
function stepNode(step, index) {
|
||||
function stepNode(job, step, index) {
|
||||
const state = nodes.runState(step);
|
||||
return node("step", `${nodes.glyphFor(state)} ${step.name || `Step ${index + 1}`}`, {
|
||||
identifier: `step-${step.number || index}`,
|
||||
identifier: `step-${job.id}-${step.number || index}`,
|
||||
description: time.duration(step.started_at, step.completed_at),
|
||||
tooltip: `${step.name || ""}\nStatus: ${state}`,
|
||||
contextValue: "step",
|
||||
@@ -39,7 +39,9 @@ function jobNode(store, repo, run, job) {
|
||||
command: "gitea.viewJobLogs",
|
||||
repo: repo,
|
||||
data: { job: job, run: run, repoKey: repo.key },
|
||||
children: steps.length ? steps.map(stepNode) : null,
|
||||
children: steps.length
|
||||
? steps.map((step, index) => stepNode(job, step, index))
|
||||
: null,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -50,18 +50,17 @@ function reviewGlyph(state) {
|
||||
return "◆";
|
||||
}
|
||||
|
||||
let counter = 0;
|
||||
|
||||
/**
|
||||
* Builds a tree node. `children` is a resolved array; `load` is an async
|
||||
* producer used for rows that fetch on expand.
|
||||
*/
|
||||
function node(kind, name, options = {}) {
|
||||
counter += 1;
|
||||
const value = {
|
||||
kind: kind,
|
||||
name: name,
|
||||
identifier: options.identifier || `${kind}-${counter}`,
|
||||
// Identifiers must be stable across rebuilds: Nova matches them to
|
||||
// work out which rows stay open when the tree reloads.
|
||||
identifier: options.identifier || `${kind}:${name}`,
|
||||
descriptiveText: options.description || "",
|
||||
tooltip: options.tooltip || "",
|
||||
image: options.image || null,
|
||||
|
||||
@@ -10,6 +10,35 @@ class NodeProvider {
|
||||
constructor(store) {
|
||||
this.store = store;
|
||||
this._roots = null;
|
||||
|
||||
// Expansion has to be tracked here. A reload rebuilds every node, and
|
||||
// the collapsible state we hand back is what Nova applies, so without
|
||||
// this every refresh would close whatever the user had opened.
|
||||
this._expanded = new Set();
|
||||
this._known = new Set();
|
||||
}
|
||||
|
||||
/** Records what the user opened or closed, keyed by node identifier. */
|
||||
setExpanded(element, expanded) {
|
||||
if (!element || !element.identifier) return;
|
||||
if (expanded) this._expanded.add(element.identifier);
|
||||
else this._expanded.delete(element.identifier);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a row should render open. A node marked `expanded` seeds the set
|
||||
* the first time it is seen, so a default-open section can still be closed
|
||||
* by hand and stay closed.
|
||||
*/
|
||||
isExpanded(element) {
|
||||
const id = element.identifier;
|
||||
if (!id) return element.expanded === true;
|
||||
|
||||
if (!this._known.has(id)) {
|
||||
this._known.add(id);
|
||||
if (element.expanded) this._expanded.add(id);
|
||||
}
|
||||
return this._expanded.has(id);
|
||||
}
|
||||
|
||||
/** Subclasses return an array of nodes (or a Promise of one). */
|
||||
@@ -55,12 +84,12 @@ class NodeProvider {
|
||||
}
|
||||
|
||||
getTreeItem(element) {
|
||||
const collapsible =
|
||||
element.children || element.load
|
||||
? element.expanded
|
||||
? TreeItemCollapsibleState.Expanded
|
||||
: TreeItemCollapsibleState.Collapsed
|
||||
: TreeItemCollapsibleState.None;
|
||||
const expandable = Boolean(element.children || element.load);
|
||||
const collapsible = !expandable
|
||||
? TreeItemCollapsibleState.None
|
||||
: this.isExpanded(element)
|
||||
? TreeItemCollapsibleState.Expanded
|
||||
: TreeItemCollapsibleState.Collapsed;
|
||||
|
||||
const item = new TreeItem(element.name, collapsible);
|
||||
item.identifier = element.identifier;
|
||||
|
||||
@@ -242,6 +242,93 @@ function declaredCommands(manifest) {
|
||||
JSON.stringify(global.__messages),
|
||||
);
|
||||
|
||||
t.section("the sidebar keeps its shape across refreshes");
|
||||
// A poll that rebuilds every row used to close whatever the user had open.
|
||||
const workflowsView = views["gitea.workflows"];
|
||||
const workflowsProvider = workflowsView.dataProvider;
|
||||
|
||||
const group = (await workflowsProvider.getChildren(null)).find(
|
||||
(n) => n.kind === "workflow" && n.name === "ci.yml",
|
||||
);
|
||||
const openMe = (await workflowsProvider.getChildren(group)).find((n) => n.data.run.id === 101);
|
||||
|
||||
t.equals(
|
||||
"a run starts collapsed",
|
||||
workflowsProvider.getTreeItem(openMe).collapsibleState,
|
||||
TreeItemCollapsibleState.Collapsed,
|
||||
);
|
||||
|
||||
workflowsView.expand(group);
|
||||
workflowsView.expand(openMe);
|
||||
|
||||
// What a refresh does: throw the built tree away and rebuild it.
|
||||
workflowsProvider.invalidate();
|
||||
const rebuiltGroup = (await workflowsProvider.getChildren(null)).find(
|
||||
(n) => n.kind === "workflow" && n.name === "ci.yml",
|
||||
);
|
||||
const rebuiltRun = (await workflowsProvider.getChildren(rebuiltGroup)).find(
|
||||
(n) => n.data.run.id === 101,
|
||||
);
|
||||
|
||||
t.check("the rebuilt run is a different object", rebuiltRun !== openMe);
|
||||
t.equals("but carries the same identifier", rebuiltRun.identifier, openMe.identifier);
|
||||
t.equals(
|
||||
"and is still open",
|
||||
workflowsProvider.getTreeItem(rebuiltRun).collapsibleState,
|
||||
TreeItemCollapsibleState.Expanded,
|
||||
);
|
||||
t.equals(
|
||||
"its parent too",
|
||||
workflowsProvider.getTreeItem(rebuiltGroup).collapsibleState,
|
||||
TreeItemCollapsibleState.Expanded,
|
||||
);
|
||||
|
||||
workflowsView.collapse(rebuiltRun);
|
||||
workflowsProvider.invalidate();
|
||||
const closedAgain = (await workflowsProvider.getChildren(
|
||||
(await workflowsProvider.getChildren(null)).find((n) => n.name === "ci.yml"),
|
||||
)).find((n) => n.data.run.id === 101);
|
||||
t.equals(
|
||||
"closing it sticks too",
|
||||
workflowsProvider.getTreeItem(closedAgain).collapsibleState,
|
||||
TreeItemCollapsibleState.Collapsed,
|
||||
);
|
||||
|
||||
// A section that opens by default must still be closable.
|
||||
const branchProvider2 = views["gitea.branch"].dataProvider;
|
||||
const folder = (await branchProvider2.getChildren(null)).find((n) => n.kind === "prFolder");
|
||||
t.equals(
|
||||
"a default-open folder starts open",
|
||||
branchProvider2.getTreeItem(folder).collapsibleState,
|
||||
TreeItemCollapsibleState.Expanded,
|
||||
);
|
||||
views["gitea.branch"].collapse(folder);
|
||||
branchProvider2.invalidate();
|
||||
const folderAgain = (await branchProvider2.getChildren(null)).find((n) => n.kind === "prFolder");
|
||||
t.equals(
|
||||
"and stays closed once the user closes it",
|
||||
branchProvider2.getTreeItem(folderAgain).collapsibleState,
|
||||
TreeItemCollapsibleState.Collapsed,
|
||||
);
|
||||
|
||||
t.section("idle polls do not redraw");
|
||||
const first = store.signature();
|
||||
t.equals("an unchanged store yields an unchanged signature", store.signature(), first);
|
||||
|
||||
const trackedRuns = store.runsFor(store.repos[0].key).runs;
|
||||
const finished = trackedRuns.find((run) => run.id === 103);
|
||||
const previousConclusion = finished.conclusion;
|
||||
finished.conclusion = "success";
|
||||
t.check("a changed run conclusion changes it", store.signature() !== first);
|
||||
finished.conclusion = previousConclusion;
|
||||
|
||||
t.equals("and restoring it restores the signature", store.signature(), first);
|
||||
|
||||
const branchFilterBefore = store.signature();
|
||||
nova.workspace.config.set("gitea.branchFilter", "all");
|
||||
t.check("changing the branch filter changes it", store.signature() !== branchFilterBefore);
|
||||
nova.workspace.config.set("gitea.branchFilter", "current");
|
||||
|
||||
t.section("api paths exercised");
|
||||
console.log([...new Set(fake.calls.map((call) => call.split("?")[0]))].sort().join("\n"));
|
||||
|
||||
|
||||
+9
-1
@@ -64,9 +64,17 @@ global.TreeView = class TreeView {
|
||||
this.dataProvider = options.dataProvider;
|
||||
this.visible = true;
|
||||
this.selection = [];
|
||||
this.reloads = 0;
|
||||
this._onExpand = [];
|
||||
this._onCollapse = [];
|
||||
treeViews.push(this);
|
||||
}
|
||||
reload() { return Promise.resolve(); }
|
||||
reload() { this.reloads += 1; return Promise.resolve(); }
|
||||
onDidExpandElement(cb) { this._onExpand.push(cb); return { dispose() {} }; }
|
||||
onDidCollapseElement(cb) { this._onCollapse.push(cb); return { dispose() {} }; }
|
||||
// Test helpers: pretend the user opened or closed a row.
|
||||
expand(element) { for (const cb of this._onExpand) cb(element); }
|
||||
collapse(element) { for (const cb of this._onCollapse) cb(element); }
|
||||
dispose() {}
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user