Repository navigation
Use shared Capture Composer in Detail with merged LUI input layout - #60
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b36e22df55
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ; modal = | ||
| (if state.modal = Append_sheet && completed_active then No_modal else state.modal) |
There was a problem hiding this comment.
When a Detail picker returns accepted items together with a partial-selection error, Capture_assets_picked stores that failure in the global capture_error; a successful Child_created closes the append composer here without clearing or retiring the error. Because both the Detail and Timeline composers subsequently render that same field, reopening either composer shows a stale failure belonging to the completed draft. Associate the error with its composer_owner and clear the matching error when that child completes.
Useful? React with 👍 / 👎.
| let owner = | ||
| Option.value (current_composer_owner state) ~default:(picker_owner state) |
There was a problem hiding this comment.
After the native picker dismisses, staging large files or photos continues asynchronously. If the user closes composer A and opens composer B before that task emits its batch, this prefers B over capture_pick_owner; the request ID consequently becomes 0, and JournalAssetImport.swift cancels the in-flight task when its request ID changes. The valid selection for A then disappears without feedback, so while a picker is armed the adapter should remain bound to the stored picker owner until a batch, dismissal, or failure terminates that request.
Useful? React with 👍 / 👎.
| Drafts.fold | ||
| (fun _ draft items -> | ||
| List.rev_append (Journal_detail.retained_attachments draft.composer) items) | ||
| t.drafts |
There was a problem hiding this comment.
When a Detail root has a canceled draft with staged attachments, stage_delete deliberately moves its composer into t.drafts so Undo can restore it. If the deletion commits, however, Subtree_deleted only clears pending_delete; it never removes that retained draft or discards the staged files collected here. Repeatedly deleting roots with attachment drafts therefore leaves stale drafts and temporary journal-import-* copies until the graph or account is torn down; add terminal-delete cleanup while preserving the existing Undo path.
Useful? React with 👍 / 👎.
| ; capture_imports = | ||
| (match imports with | ||
| | None -> state.capture_imports | ||
| | Some (batch_target, batch_items) -> | ||
| Some | ||
| { batch_generation = state.graph_state.generation | ||
| ; batch_target | ||
| ; batch_items | ||
| }) |
There was a problem hiding this comment.
If child creation succeeds but one of these queued attachment imports later fails, the import runner discards the staged file and tells the user to select it again. This change also removes the only direct Detail attach action, so selecting the file again through Capture creates another child (or a grandchild after opening the new block) rather than retrying the import into batch_target. Retain enough target state to retry the failed import against the created child, or expose a direct attach action for that target.
Useful? React with 👍 / 👎.
RCmerci
changed the title
1 participant
Detail exposes separate Append and Attach file toolbar actions. This change gives Detail the same bottom-right Capture control and shared Composer as Timeline. Send appends one last child under the owned Detail root, then imports its selected attachments serially into that child. Draft text and files survive cancel/reopen and navigation; duplicate submission and stale completion remain fenced.
Both Journal dependency manifests now pin the merged LUI revision
8de2abae573499692dbcc169a2738c434944a0a8from logseq/lui#172. Its Composer gives text its own full-width growing row (36–168pt), with attachments and controls on separate rows. Native Detail Chrome uses one body slot to avoid an empty toolbar placeholder.The review regressions are fixed: picker errors belong to their Composer; asynchronous selection keeps its original owner through navigation; committed root deletion retires retained draft files after Undo is no longer available; failed attachment imports retain their bytes and offer Retry imports against the already-created child. Explicit Dismiss and account/graph retirement release failed-import bytes.
Validation:
dune build @all app/native_embed.exe.oand the fulldune runtest --forcepass, including 74 Application tests and the public Routes ownership/deletion regression.Exact head
5e7761dbb7b5e811a738a79431a5ec50f569fc60: Journal CI completed successfully, including locked dependency installation, all-target/native-embed build, and full regressions. CI dependency receipts resolve LUI to the same8de2abae573499692dbcc169a2738c434944a0a8revision.