Skip to content

feat(vault): trash/download actions + path-bar theming seam#177

Merged
vutuanlinh2k2 merged 1 commit into
mainfrom
feat/vault-actions-pathbar
Jul 9, 2026
Merged

feat(vault): trash/download actions + path-bar theming seam#177
vutuanlinh2k2 merged 1 commit into
mainfrom
feat/vault-actions-pathbar

Conversation

@vutuanlinh2k2

Copy link
Copy Markdown
Contributor

Summary

Improves the vault path row and exposes a theming seam for host products.

Changes

  • Delete control now uses a red Trash2 icon (was a plain glyph).
  • Download action added in the open file's path row, before delete, driven by a new onDownloadFile callback. Omit the callback to hide the button (e.g. when a custom artifact renderer provides its own download affordance).
  • Path row background is configurable via pathBarClassName (defaults to bg-card) so hosts can align it with their artifact chrome.
  • Open file path rendered bolder (font-medium text-foreground).

Contract

Adds onDownloadFile?: (file: VaultFile) => void and pathBarClassName?: string to VaultPaneProps (both optional; defaults preserve current behavior).

Notes

Consumed by GTM Agent's vault (separate PR).

- Delete control uses a red Trash2 icon (was a plain glyph).
- Add a download action in the open file's path row, before delete,
  driven by a new onDownloadFile callback (omit to hide it).
- Path row background is configurable via pathBarClassName (defaults to
  bg-card) so hosts can align it with their artifact chrome.
- Open file path rendered bolder (font-medium text-foreground).

Adds onDownloadFile and pathBarClassName to VaultPaneProps.
@vutuanlinh2k2 vutuanlinh2k2 requested a review from tangletools July 9, 2026 08:50

@tangletools tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Value Audit — sound-with-nits

Verdict sound-with-nits
Concerns 1 (1 weak-concern)
Heuristic 0.0s
Duplication 0.0s
Interrogation 170.5s (2 bridge agents)
Total 170.5s

💰 Value — sound-with-nits

Adds a download action, clearer delete icon, and a path-bar theming seam to VaultPane — small, optional, and consistent with the shell's seam-driven design.

  • What it does: In VaultPane, the open file's path row now renders a Download button (before delete) when the host supplies onDownloadFile; the delete affordance is now a red Trash2 icon instead of a raw glyph; the path text is bolder (font-medium text-foreground); and the path-row background defaults to bg-card but can be overridden via pathBarClassName. Two optional props are added to `VaultPa
  • Goals it achieves: Let host products offer a one-click download from the vault chrome without re-implementing the path row; let hosts visually blend the path bar with their own artifact chrome; and make destructive/delete affordance more recognizable.
  • Assessment: Good change. It is additive, optional, and narrow in scope. It follows the existing shell pattern of typed seams (VaultDataPort, renderTree, renderArtifact, headerActions) and uses the repo's already-present lucide-react icon library and shared theme tokens (bg-card, text-foreground, text-destructive). Defaults preserve prior behavior for callers that omit the new props.
  • Better / existing approach: none — this is the right approach. The download action belongs in the chrome the pane owns, not inside the product-supplied renderArtifact seam, and a callback is the lightest way to hand the actual blob/mime logic to the host. No existing path-bar action slot or theming hook was found (grep for pathBarClassName, onDownloadFile, download in src/vault shows only this change).
  • Model: opencode/kimi-for-coding/k2p7
  • Bridge attempts: 1

🎯 Usefulness — sound

Clean additive seam — two optional props (download callback + path-bar class) that follow the vault module's established product-supplied-chrome pattern, with an imminent external consumer.

  • Integration: Reachable and wired correctly. Both props are destructured from VaultPaneProps (VaultPane.tsx:190-191) and rendered inside the existing selectedFile path-row block (lines 507, 557-566). The vault surface is exported via ./vault and ./vault/lazy (package.json:312-320, tsup.config.ts:59-60, src/vault/index.ts:13-14), and VaultPaneProps is re-exported from both, so any consumer of the vault sur
  • Fit with existing patterns: Matches the codebase grain exactly. The precedent is headerActions?: ReactNode (contracts.ts:152) — an optional product-supplied chrome hook in the same pane. onDownloadFile is the callback variant of the same idea (product owns behavior, pane owns button chrome), and pathBarClassName mirrors the existing className interpolation pattern (VaultPane.tsx:451). Both default-preserving and addi
  • Real-world viability: Holds up off the happy path. Null-safe: onDownloadFile(selectedFile) (line 562) sits inside the {selectedFile && (...)} guard (line 506), so no null deref. Correct gating: the download button is NOT gated by canWrite (download is a read op, available in read-only vaults) nor by isMarkdownCapable (download applies to any file type) — both are the right semantics, while delete stays gated by
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

💰 Value Audit

🟡 pathBarClassName replaces the default background entirely [better-architecture] ``

The implementation defaults to bg-card only when pathBarClassName is omitted; if a host passes a className, they must re-specify the background to keep it (src/vault/VaultPane.tsx:507). A small API refinement would be to make pathBarClassName additive (with !bg- or a dedicated pathBarBackground color token) so hosts can override just the surface without re-carrying layout defaults. This does not block shipping.


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260709T085852Z

@vutuanlinh2k2 vutuanlinh2k2 merged commit 3136e83 into main Jul 9, 2026
1 check passed
@vutuanlinh2k2 vutuanlinh2k2 deleted the feat/vault-actions-pathbar branch July 9, 2026 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants