Add file-picker element (files/photos/camera) for the Apple backend - #73
Conversation
Introduces a generic non-visual 'file-picker' node kind driven by the
schema: a 'request' token presents a picker for 'source'
(files/photos/camera), 'picked' emits a JSON payload echoing the token
plus per-file {path, name, content-type}, 'completion' releases retained
security-scoped resources, and a cancelled presentation reports through
'dismiss'. 'types' takes a comma-separated UTI list and 'multiple'
enables multi-select.
Apple backend mounts fileImporter, SwiftUI PhotosPicker, or an iOS-only
UIImagePickerController camera wrapper; macOS camera requests answer
with dismiss. Picked URLs keep their security scope (or temp copies for
photo/camera results) until completion echoes the token or the node is
dropped, and the iOS fileImporter never-calls-on-completion-on-cancel
quirk is covered by presentation-state cancel detection.
Flutter, Qt, and WinUI get schema parity and stub rendering only.
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
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. |
…visible-range tracking Schema gains five node kinds (list-section, list-section-header, list-section-footer, swipe-actions, swipe-action), nine properties (key, separator, style, scroll-target, scroll-anchor, scroll-token, scroll-animated, track-visible-range, edge), and two list events (scroll-completed, visible-range). OCaml: list_item gains ?expanded/?on_toggle disclosure support plus ?separator/?swipe_actions; list gains ?style, scroll request props and ?on_scroll_completed/?on_visible_range; new list_section, swipe_actions and swipe_action constructors. Apple backend: list-section children render as grouped sections with arbitrary content headers/footers (existing heading/footnote inference still applies when no explicit sections are present); a list-item with children renders a DisclosureGroup; explicit swipe-actions render on both edges while the context-menu trailing-swipe derivation is preserved; scroll-target/scroll-token drive ScrollViewReader requests reporting scroll-completed outcomes; track-visible-range emits debounced flat-position visible-range events. Flutter/Qt/WinUI gain schema-parity validation; rendering stays a container fallback.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef52d08770
ℹ️ 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".
| func makeUIViewController(context: Context) -> UIImagePickerController { | ||
| let controller = UIImagePickerController() | ||
| controller.sourceType = .camera | ||
| controller.mediaTypes = mediaTypes.map(\.identifier) |
There was a problem hiding this comment.
Check camera availability before assigning the source
When source == "camera" is used on an iOS Simulator, a device without a camera, or a device where camera capture is unavailable, assigning .camera to sourceType raises an Objective-C exception instead of producing the documented dismiss event. Check UIImagePickerController.isSourceTypeAvailable(.camera) before presenting and cancel the request when it is unavailable.
Useful? React with 👍 / 👎.
| private static func importPhotoItem(_ item: PhotosPickerItem) async -> RetainedFile? { | ||
| guard let data = try? await item.loadTransferable(type: Data.self) else { | ||
| return nil | ||
| } | ||
| let ext = item.supportedContentTypes.first?.preferredFilenameExtension ?? "bin" | ||
| let url = FileManager.default.temporaryDirectory | ||
| .appendingPathComponent("lui-picked-\(UUID().uuidString)") | ||
| .appendingPathExtension(ext) | ||
| guard (try? data.write(to: url)) != nil else { return nil } |
There was a problem hiding this comment.
Transfer photo-library movies without loading them into memory
With the default photo filter, users can select movies, and loadTransferable(type: Data.self) materializes the entire asset in process memory before writing another copy. A normal multi-gigabyte phone video can therefore terminate the app for memory pressure; use a file-based Transferable/FileRepresentation or otherwise stream/copy from a supplied URL.
Useful? React with 👍 / 👎.
| private func finishCancelled() { | ||
| handled = true | ||
| releaseRetained() | ||
| operation = nil | ||
| try? backend.performDismiss(node: model.id) |
There was a problem hiding this comment.
Clear presentation state when camera capture is cancelled
When the camera controller calls imagePickerControllerDidCancel, the callback reaches finishCancelled(), but this path never sets presented to false. UIImagePickerController delegates are responsible for dismissing the picker, so the camera sheet can remain visible after the dismiss event has already been sent, with operation cleared and no way to complete that presentation.
Useful? React with 👍 / 👎.
| .onAppear { | ||
| evaluate() | ||
| evaluateCompletion() | ||
| } |
There was a problem hiding this comment.
Honor an existing completion token before presenting
If a picker mounts with request and completion already equal—for example when application state retaining the last handshake recreates the node—evaluate() first sets presented = true, and evaluateCompletion() only clears operation. The acknowledged request consequently opens another picker whose result is ignored; completion must be evaluated first or presentation must explicitly exclude already-completed tokens.
Useful? React with 👍 / 👎.
| #endif | ||
| finishCancelled() | ||
| } | ||
| .onDisappear { releaseRetained() } |
There was a problem hiding this comment.
Retain picked files across temporary view disappearance
If the picker view temporarily disappears while its backend node remains mounted, such as when switching tabs or navigation destinations before asynchronous processing acknowledges completion, this hook immediately closes security scope and deletes photo/camera temp files. That invalidates the paths despite the API promise to retain them until matching completion or node removal; cleanup needs to follow the node/request lifecycle rather than SwiftUI visibility.
Useful? React with 👍 / 👎.
| private func handlePhotoSelection(_ items: [PhotosPickerItem]) { | ||
| guard operation != nil, source == "photos", !items.isEmpty else { return } | ||
| photoSelection = [] | ||
| Task { |
There was a problem hiding this comment.
Mark photo selections handled before clearing the binding
When a photo selection closes the picker, SwiftUI updates photoSelection and presented in the same render cycle. This handler clears photoSelection while handled is still false, so the outer onChange(of: presented) can observe an empty selection and call finishCancelled() before the asynchronous import completes; the eventual finishPicked() then sees no operation and drops the selected files. Mark the operation as handling a selection before clearing the binding.
Useful? React with 👍 / 👎.
| // Non-visual node: presentation is driven by its properties on | ||
| // platforms that implement the file-picker backend; children render | ||
| // inline. | ||
| _NodeKind.filePicker => Stack(children: children), |
There was a problem hiding this comment.
Dismiss unsupported picker requests on non-Apple backends
When an app sends a camera request on Flutter, this new branch only renders a Stack and never emits Dismiss, leaving the request pending forever despite the public API explicitly promising that non-iOS platforms answer camera requests with a dismiss event. The WinUI and QML placeholders added by this commit are likewise inert, so unsupported hosts need to observe new request tokens and dispatch the documented fallback event.
Useful? React with 👍 / 👎.
| ?request:string -> | ||
| ?request_signal:string Signal.signal -> |
There was a problem hiding this comment.
Expose integer request tokens through the element API
The wire schema and every backend explicitly accept int as well as string values for request, and the picked payload promises to preserve that token type, but the public file_picker constructor restricts both its static and signal request arguments to strings. Apps whose operation IDs are integers therefore cannot use the advertised token form through the typed element API and must bypass it with raw property calls; expose a token type that covers both supported wire values, with the same treatment for completion.
Useful? React with 👍 / 👎.
|
End-to-end verified on macOS with a scratch SwiftUI harness mounting a real
Recording: /Users/devin/screencasts/rec-5039b89a-2e62-495e-be0d-ce7e34230d4f/rec-5039b89a-2e62-495e-be0d-ce7e34230d4f-edited.mp4 Not covered: |
- Hold file-picker operations on the backend keyed by node id so view teardown can't release retained files; drop-node releases them. - Stream PhotosPicker items through FileRepresentation instead of loading whole assets into memory as Data. - Guard UIImagePickerController.isSourceTypeAvailable(.camera) — a camera-less device now answers dismiss instead of throwing. - Mark photo selections handled before clearing so async imports are not mistaken for cancels; finishCancelled also tears down the camera sheet; requests equal to completion no longer re-present. - Flutter/WinUI backends answer unservable file-picker requests with dismiss after the batch commits; the Qt stub does the same in QML. - Lui_elements.file_picker request/completion take a file_picker_token ([`String | `Int]) matching the wire's string|int property values.
…ction handling
- Apple: emit visible-range on flatRowOrder changes (insert/remove/reorder
with same visible ids); wrap headerless explicit sections in Section{};
apply heading/footnote inference to pending non-section runs; resolve
scroll targets by section key; guard fallback cancelled emission with
handledScrollToken; keep context-menu ellipsis when explicit
swipe-actions exist.
- Flutter: exclude swipe-actions children from list-item row content.
- Web: treat swipe-actions as hidden metadata in list-item content
validation and DOM placement (never mounted).
- Qt/WinUI: require text or icon on swipe-action for parity.
…mber picked event code to 13
An empty ForEach mounts no view, so .fileImporter/.photosPicker/.sheet on LUIFilePickerView never presented when the node had no children.
Summary
Adds a general-purpose non-visual
file-pickerelement to lui, covering the same contract as the journal'sjournal-asset-importextension (file dialog pick + security-scope retention + completion handshake) plus photo-library and camera sources — all with generic naming so any app can drive it. The token/payload contents stay app-side; no journal semantics are baked into the schema or backends.Wire surface (from
schema/components.json, regenerated):file-picker(container, mounted as a child likesheet/dialog; non-visual)request(string|int token — set/bump to present),types(comma-separated UTIs),multiple(bool),source("files"default /"photos"/"camera"),completion(string|int token), plusenabledandappear-enabled(restrictive kind)picked(node, payload: string); cancelled presentations reusedismissHandshake: when
requestchanges to a new token andenabledholds, the platform picker presents. On success,pickedfires with{"request": <token>, "files": [{"path", "name", "content-type"}]}(token type preserved;filessupports multi-select). On cancel,dismissfires — including the iOS quirk where.fileImporternever invokes its completion on cancel, detected by watching the presentation flip while unhandled. Picked URLs retain their security scope keyed by the request token untilcompletionechoes it or the node drops; photo/camera results are temp copies owned by the picker and deleted on release.Apple backend sources:
files→.fileImporterwithallowedContentTypesparsed fromtypesphotos→ SwiftUIphotosPicker(PHPickerFilternarrowed bytypes)camera→UIImagePickerController(.camera)viaUIViewControllerRepresentable, iOS only; on macOS acamerarequest answers withdismisswithout presentingLui_elements.file_pickerexposes?source ?request ?types ?multiple ?disabled ?completion ?on_picked ?on_dismiss(with signal twins) overLui_protocol.PickerRequest/…/Picked. Flutter gets the prop/event schema +pickedbridge + aStackstub; Qt a stub QML + schema arms; WinUILUIEvent.Picked/PerformPicked+ schema parity. The example OCaml bridges and Swift hosts gainpickedplumbing.Test plan
dune build,dune runtest— 30 tests incl. newfile-picker rules(kind/property/event matrix, element wiring,picked/dismissdispatch)swift test— 138 tests incl. newLUIFilePickerTests(prop validation,performPicked/performDismisskind gating)dart analyze+flutter test— clean; Qt lib + test target compile under CMakeLink to Devin session: https://app.devin.ai/sessions/ae0d876b6fc4405487695d8ebd90712b
Open in Devin Desktop: https://app.devin.ai/desktop/session/ae0d876b6fc4405487695d8ebd90712b?variant=devin
Requested by: @RCmerci