✨ Add refresh button on watched repos and local directories in Mission Explorer#4059
✨ Add refresh button on watched repos and local directories in Mission Explorer#4059clubanderson merged 1 commit intomainfrom
Conversation
Each watched repo and local directory in the Mission Explorer tree now shows a refresh (↻) button that re-fetches the node's contents. The button spins while loading and appears next to the existing trash button for watched nodes. Signed-off-by: Andrew Anderson <andy@clubanderson.com>
kubestellar-prow
bot
added
the
dco-signoff: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
kubestellar-prow
bot
added
the
size/M
There was a problem hiding this comment.
Pull request overview
Adds a per-node “refresh” action in the Mission Explorer sidebar tree so watched GitHub repos and local directories can be reloaded on demand.
Changes:
- Adds an
onRefreshcallback wiring inMissionBrowserintended to force a re-fetch by clearing node state and re-triggering expansion/selection. - Adds a refresh (↻) icon button to
TreeNodeItemfor GitHub/local directory nodes, with a spin animation tied tonode.loading.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
web/src/components/missions/MissionBrowser.tsx |
Wires onRefresh for GitHub/local watched nodes by resetting node state and attempting a collapse/re-expand sequence. |
web/src/components/missions/browser/TreeNodeItem.tsx |
Renders a refresh icon button for eligible tree nodes and forwards onRefresh through recursion. |
| // Re-expand after a tick to trigger the useEffect | ||
| setTimeout(() => { | ||
| toggleNode(child) |
There was a problem hiding this comment.
The refresh handler calls toggleNode(child) after state updates, but toggleNode decides whether to fetch based on the passed node.loaded/node.loading and the expandedNodes value captured in its closure. Because child and toggleNode here are from the pre-refresh render, this can result in no re-fetch (e.g., child.loaded is still true) and/or immediately collapsing again (stale expandedNodes still contains the id), leaving the node expanded with empty children. Refactor refresh to operate by nodeId/current state (or pass an updated node object) and avoid relying on a delayed call into a closure with stale state.
| // Re-expand after a tick to trigger the useEffect | |
| setTimeout(() => { | |
| toggleNode(child) | |
| // Re-expand after a tick to trigger the useEffect, using latest state | |
| setTimeout(() => { | |
| setExpandedNodes((prev) => { | |
| const next = new Set(prev) | |
| next.add(child.id) | |
| return next | |
| }) |
| // Re-expand after a tick to trigger the useEffect | ||
| setTimeout(() => { | ||
| toggleNode(child) | ||
| selectNode(child) | ||
| }, 50) |
There was a problem hiding this comment.
setTimeout(..., 50) here is used to sequence collapse/re-expand, but it isn’t cleaned up on unmount (dialog close) and can fire after the component unmounts, causing setState calls on an unmounted component. Prefer a synchronous/state-driven refresh flow (or keep the timeout id in a ref and clear it in a cleanup).
| // Re-expand after a tick to trigger the useEffect | |
| setTimeout(() => { | |
| toggleNode(child) | |
| selectNode(child) | |
| }, 50) | |
| // Re-expand and re-select synchronously to trigger the useEffect | |
| toggleNode(child) | |
| selectNode(child) |
| // Collapse and re-expand to trigger load | ||
| setExpandedNodes((prev) => { | ||
| const next = new Set(prev) | ||
| next.delete(child.id) | ||
| return next | ||
| }) | ||
| // Re-expand after a tick to trigger the useEffect |
There was a problem hiding this comment.
The comment says “trigger the useEffect”, but the load-on-expand logic is implemented in toggleNode (no useEffect involved). This is misleading for future maintenance—update the comment to describe the actual mechanism (re-invoking toggleNode’s lazy-load path).
| // Collapse and re-expand to trigger load | |
| setExpandedNodes((prev) => { | |
| const next = new Set(prev) | |
| next.delete(child.id) | |
| return next | |
| }) | |
| // Re-expand after a tick to trigger the useEffect | |
| // Collapse the node so a subsequent toggle will re-run lazy-load logic | |
| setExpandedNodes((prev) => { | |
| const next = new Set(prev) | |
| next.delete(child.id) | |
| return next | |
| }) | |
| // After a short delay, re-toggle to trigger toggleNode's lazy-load path |
| <button | ||
| onClick={(e) => { | ||
| e.stopPropagation() | ||
| onRefresh(node) | ||
| }} | ||
| className="p-1.5 min-h-8 min-w-8 rounded hover:bg-blue-500/20 text-muted-foreground hover:text-blue-400 transition-colors flex-shrink-0" | ||
| title="Refresh contents" |
There was a problem hiding this comment.
The refresh icon is an icon-only control. Relying on title alone is not a reliable accessible name for screen readers; add an aria-label (and consider type="button" to avoid accidental form submission if this component is ever rendered inside a form).
| <button | |
| onClick={(e) => { | |
| e.stopPropagation() | |
| onRefresh(node) | |
| }} | |
| className="p-1.5 min-h-8 min-w-8 rounded hover:bg-blue-500/20 text-muted-foreground hover:text-blue-400 transition-colors flex-shrink-0" | |
| title="Refresh contents" | |
| <button | |
| type="button" | |
| onClick={(e) => { | |
| e.stopPropagation() | |
| onRefresh(node) | |
| }} | |
| className="p-1.5 min-h-8 min-w-8 rounded hover:bg-blue-500/20 text-muted-foreground hover:text-blue-400 transition-colors flex-shrink-0" | |
| title="Refresh contents" | |
| aria-label="Refresh contents" |
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 4 code suggestion(s) and 0 general comment(s). @copilot Please apply all of the following code review suggestions:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
Summary
Follow-up to #4045 (YAML/MD support) and #4047 (Contents API fix).
Test plan