Repository navigation
Conversation
…ard state First sub-key: filesDirty (tracked file, dirty flag, pending discard decision). One sub-key per guarded surface; agents-form and workflows join here later. Ephemeral by design, never persisted.
…raction store editorDirtyRef (a ref mirroring editor dirty state) is gone; the page reads dirty/fileName from the filesDirty sub-key and the blocker path prompts/answers through store actions. Pure helpers in files-dirty-guard keep their signatures (store-agnostic). Wiring pins updated.
…ction store The editor no longer owns a dirty useState: it reports buffer divergence through the store (setFilesDirty) and reads the flag back for the modified badge. The saved-text baseline stays editor-local (mechanics, not protocol). Workflow-edit-panel keeps its dirty prop contract via onDirtyChange, so no changes there.
…e type First modularization commit (plan: llm-steering sessions/files-page- modularization-plan.md). Pure leaves first: fmtSize/fmtMtime into pages/files/format.ts with behavioral tests, Notice type into pages/files/notice.ts. No logic changes.
…ateway preview-pane.tsx moves out verbatim (stateless, dirty via injected callback); the in-shell blob-download helper becomes save-via-gateway.ts (module-level object-URL bookkeeping is download mechanics, not page state). Page shrinks 864→752.
Row rendering, selection, activation and drag/drop move to files/file-table.tsx as a stateless component emitting events; the page keeps guard wiring and all state. The dir-drop upload becomes uploadInto(dirPath, files) at page level, reusing the same 409-overwrite flow as toolbar upload. Drops the wiring-pin test file that returned with main's history — behavior lives in the pure-module and store tests.
…lbar Both stateless: toolbar takes filter/sort state pairs and action callbacks; the rename button's entry lookup moves inside the toolbar (props carry entries). Notice bar rides with the toolbar. Page 604→496.
All mutation ops (upload with 409-overwrite, mkdir, rename, delete with recursive confirm, move, copy paths/URLs) plus busy/notice state move to files/use-file-ops.ts. The hook takes injected confirm/prompt functions and preview-invalidation callbacks so the page keeps owning previewPath and both confirm-dialog instances. index.tsx is now a composition root: state, guard wiring, child mount (~344 lines, from 864).
The inline shouldBlockFn returned the raw confirm result, so a cancelled discard (ok=false) let the navigation proceed. Delegate to the tested shouldBlockFilesLeave helper, which applies the !ok blocker polarity, and read the ui-interaction store imperatively so the blocker callback keeps a stable identity. Includes [FILES] console probes and window.__router debug hook used to hand-trace the navigation stall — to be stripped before push.
… state to a view store Stage 1 of the files-page simplification: filter, sort key, selection set, and previewPath move out of FilesPage's useState hooks into a dedicated files-view-store (zustand), same pattern as the dirty-state unification. FilesPage subscribes to the four values and calls actions imperatively; toggleSelected replaces the inline Set-copy logic. No behavior change.
… view store openEntry read the dirty flag from its render closure, but the callback is memoized on deps that never include `dirty`, so the captured value stayed false after typing and guardOpenFile short-circuited: clicking a second file replaced the preview with no discard prompt, while the router blocker — which reads the store at call time — prompted correctly for directories. Read the store at call time here as well. The same stale capture could also go the other way, prompting to discard a file that was already clean. Also folds the notice bar into the files view store (refactor plan stage 2 lists it as a store member; stage 1 drops the PreviewPane onNotice prop): FilesToolbar and PreviewPane subscribe to the slice instead of taking notice/onNotice props, and the page clears the bar on unmount because the store outlives the page.
…irm dialog per page The toolbar took `entries` + `selected` only to resolve the single entry Rename acts on. The page now resolves that once and passes `renameTarget?: FileEntry`, and the toolbar reads `selected` straight from the view store for the action-row disabled states. Rename is disabled whenever no target resolves, where before it was enabled on a stale selection and the click silently did nothing. files.tsx also held two `useConfirmDialog()` instances — the ops confirm/prompt queue and the dirty-guard discard queue — so two modals could be open at once. One instance now serves both and renders a single dialog element. Sort stays typed as SortKey, but the `as SortKey` cast only moves from the page into the toolbar: `Select` is string-typed and not generic, so the conversion has to happen at the widget boundary somewhere. Typing Select would touch every page that uses it.
Resolving the rename target from the first selected name meant any multi-select produced a target, so Rename enabled with two or more rows checked and would have renamed an arbitrary one of them. Require exactly one selection, as the toolbar did before it took a resolved target instead of the selection itself.
| @@ -4,78 +4,29 @@ | |||
| * upload into the current dir, and drag rows onto folders to move. | |||
| */ | |||
|
|
|||
There was a problem hiding this comment.
This one still looks horrendous but the goal is for it to look like breadcrumbs.tsx, file-table.tsx, or any of the other bits that got componentized away: markup sprinkled with code, preceded by some simple variable declaration and small helper functions. All the state management goes to reducers and all the state is held in zustand, which we already do for a lot of non-UI stuff.
| * emits `onOpenEntry` / `onOpenRaw` / `onMoveOntoDir` / `onDropFilesInto`. | ||
| */ | ||
| export function FileTable(props: { | ||
| path: string |
There was a problem hiding this comment.
I'm not in love with the number of arguments in some of these. The hope is, once this is modularized, it should be much easier to analyze which of these are truly essential, which can be further componentized away, etc.
|
|
||
| export type SortKey = 'name' | 'mtime' | 'size' | ||
|
|
||
| interface FilesViewState { |
There was a problem hiding this comment.
So this is where all the mess of event propagation that was causing the cross-linked bugs with dirty states in file editor, and each visual component having its own event handlers, some cancelling others, etc went away to: the FileViewState object contains every aspect of what's happening with this visual component, and it gets folded into the Zustand state with a reducer. All decisions about what to ask the user about what, what actions should be allowed, etc are made in one place.
rivetphilbot
left a comment
rivetphilbot
left a comment
There was a problem hiding this comment.
Direction looks right: one view store, one ops hook, stateless components, and the dirty guard out of the component tree. Three things before this can go green and come out of draft:
- CI typecheck is red.
workflow-edit-panel.tsx:325still passesonDirtyChangetoFileEditor, and this PR removed that prop. See the inline note onfile-editor.tsxfor a fix that also closes a cross-surface leak. - Debug leftover in
main.tsx(window.__router). Inline note. - Housekeeping: branch is 5 behind
main, please rebase. The title should be a conventional commit, e.g.refactor(rivethub-web): files page view store, ops hook, stateless components. The lint job reports 19 warnings; worth a look to confirm none are new in the touched files.
Happy to re-review once it's green.
| // Dirty is protocol state — it lives in the ui-interaction store keyed by | ||
| // the tracked file; the editor only reports divergence and reads it back | ||
| // for the modified badge. | ||
| const dirty = useUiInteraction((s) => s.filesDirty.dirty) |
There was a problem hiding this comment.
Two problems with reading filesDirty here.
FileEditoris also mounted byworkflow-edit-panel.tsx, which still passesonDirtyChange(that's the CI typecheck failure).- Every
FileEditorinstance now reads and writes the singlefilesDirtyslot, so the workflow panel's editor would flip the Files page dirty flag and show the Files page badge. That is the same cross-surface coupling this PR is removing, just moved into the store. The store header already says one sub-key per guarded surface, so the editor shouldn't pick a surface itself.
Suggest keeping the editor stateless about which surface it belongs to: keep onDirtyChange?: (dirty: boolean) => void on FileEditorProps (and a local dirty for the badge, or a dirty?: boolean prop), and have the Files page / PreviewPane wire onDirtyChange={setFilesDirty} from useUiInteraction. The workflow panel keeps working unchanged, the Files page still owns its dirty state in the store, and the editor has no store dependency.
|
|
||
| const router = createRouter({ routeTree }) | ||
| // Debug: expose the router for hand-tracing navigations in devtools. | ||
| ;(window as unknown as { __router?: unknown }).__router = router |
There was a problem hiding this comment.
Drop this before merge; it ships a global in the production bundle. If it's useful while tracing, gate it behind import.meta.env.DEV in a local branch rather than committing it.
2 participants
Description
Reactoring the web code: modularizing JSX, using existing zustand state manager with reducers for state, making components stateless
Related Issue
Closes #
Checklist
@rivetos/typesonly)feat:,fix:,docs:,test:,chore:)npm testpasses