media: file-image, file-preview (QuickLook), and link elements - #71
Conversation
Three general-purpose element kinds sufficient to replace journal-media style per-app extensions: - file-image: loads a local path / file:// URL off the main actor via CGImageSource thumbnail decoding (NSCache keyed by path+pixel size, ~32MB cap), with loading spinner, failure icon fallback, and the standard press-enabled/Press path. - file-preview: a non-rendering node that presents QuickLook via .quickLookPreview. Presence in the tree means presented, matching the sheet/dialog 'mount to present' pattern but on its own presentation lane (not isModalSurface, which forces a text title and sheet chrome). Interactive close emits Dismiss and is suppressed from re-asserting until the wire drops the node, same as modal dismissal. - link: a container rendering SwiftUI Link(destination:) from a url prop; children (or text/icon props) form the label, so it composes inside list rows and overlays. New wire properties: path, url, max-pixel-size. file-preview is a restrictive kind (path + accessibility-identifier only). Flutter/Qt/ WinUI/web schema mirrors updated; Qt gains stub QML views; WinUI's restrictive matrix covers file-preview automatically.
|
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbf1db5220
ℹ️ 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".
| | Link -> "lui-link" | ||
| | FileImage -> "lui-file-image" | ||
| | FilePreview -> "lui-file-preview" |
There was a problem hiding this comment.
Handle the new properties in the web renderer
On the web backend, mounting any of these new kinds emits the required UrlValue or PathValue property, and may emit MaxPixelSize, but Lui_web_props.apply_secondary_property handles none of them and falls through to invalid_arg. Consequently, otherwise-valid link, file-image, and file-preview patches fail when their properties are applied instead of producing even a stub node; add apply/remove handlers for the new properties before exposing these kinds.
Useful? React with 👍 / 👎.
| TapHandler { | ||
| enabled: wrapper.props["press-enabled"] === true | ||
| onTapped: { | ||
| if (wrapper.node && wrapper.node.backend) | ||
| wrapper.node.backend.performPress(wrapper.node.id) |
There was a problem hiding this comment.
Allow Qt file images through performPress
When a Qt file-image has press-enabled, this tap handler calls LuiQmlBackend::performPress, but that method's pressable-kind whitelist does not include NodeKind::FileImage. Every such tap therefore returns false and emits no Press event despite the schema and OCaml on_press API advertising support; add the kind with the same press-enabled guard used here.
Useful? React with 👍 / 👎.
| private var filePreviewBinding: Binding<URL?> { | ||
| Binding( | ||
| get: { backend.filePreviewPresentation.item?.url }, | ||
| set: { url in |
There was a problem hiding this comment.
Scope Quick Look presentation to the traversal root
In a backend hosting multiple LUISwiftUIRoots—the scenario this modifier explicitly supports—every root's binding returns the same global preview URL because it ignores LUIFilePreviewPresentation.rootID. A preview mounted under one traversal root can therefore be presented by the wrong host or by several hosts, and a dismissal from any of them clears the shared store; filter the item by rootID, as sheetBinding already does, in both preview bindings.
Useful? React with 👍 / 👎.
| .task(id: "\(maxPixelSize)|\(path)") { | ||
| if let decoded = await LUIFileImageLoader.thumbnail( | ||
| path: path, | ||
| maxPixelSize: maxPixelSize | ||
| ) { | ||
| image = decoded |
There was a problem hiding this comment.
Ignore obsolete thumbnail task results
When path or maxPixelSize changes while a previous decode is running, SwiftUI cancels the old .task, but thumbnail launches an unstructured detached task and this closure neither checks cancellation nor verifies the captured key after awaiting it. The older decode can finish after the newer one and overwrite image with pixels from the previous path; guard cancellation or compare the task key before updating state.
Useful? React with 👍 / 👎.
| | Slider -> "input" | ||
| | Divider -> "hr" | ||
| | Tooltip -> "span" | ||
| | Link -> "a" |
There was a problem hiding this comment.
Build a DOM label structure for link icons
Once the required URL property is handled, a web link using the newly supported icon prop still fails: links are created as empty simple <a> elements, while apply_inline_icon_name treats every non-list/non-bottom-tab kind as a button and calls Util.button_icon_node, which requires an element child at index 0. The normal OCaml constructor emits the icon before mounting label children, so this raises an out-of-bounds error; create the expected icon/label children for links or add link-specific icon handling.
Useful? React with 👍 / 👎.
| case LUINodeKind.Text: | ||
| case LUINodeKind.TableCell: | ||
| case LUINodeKind.TimelineItem: | ||
| case LUINodeKind.FileImage: |
There was a problem hiding this comment.
Admit WinUI file images through PerformAction
When a WinUI adapter or test invokes the public core action gate for a file-image with press-enabled, LUISchema.EventSupported now advertises the event but LUIBackend.PerformAction still omits LUINodeKind.FileImage from its pressable whitelist. The call therefore throws instead of emitting LUIEvent.Press; add the same PressEnabled-guarded case used by the Apple and Flutter backends.
Useful? React with 👍 / 👎.
…vent gates - web: link renders as a structured <a> (icon + content spans) so inline icon, text, and children compose; url maps to href (+target=_blank, rel=noopener), path/max-pixel-size become data-* attributes with matching remove handlers; file-image clicks emit Press when press-enabled. - apple: both filePreviewBindings filter on the presentation's rootID and only dismiss previews owned by that root — multiple LUISwiftUIRoots no longer share/dismiss one global preview. - qt: performPress admits FileImage with the same press-enabled guard; performDismiss admits FilePreview (Dismiss is advertised for it). - winui: PerformAction admits FileImage gated on PressEnabled; PerformDismiss admits FilePreview.
…a-file # Conflicts: # platform/apple/Sources/LUIAppleBackend/LUIWireSchema.swift # platform/flutter/lib/lui_flutter_backend.dart # platform/qt/lib/CMakeLists.txt # platform/qt/lib/lui_wire_schema.h # platform/winui/LUI.Core/LUISchema.cs # platform/winui/LUI.Core/LUIWireSchema.g.cs # schema/components.json # src/lui_protocol.ml # src/lui_protocol.mli # src/lui_wire_schema.ml # test/test_lui.ml
Summary
Adds three general-purpose element kinds so apps no longer need private extensions for file-backed media and external links (motivating case: replacing
JournalMedia.swift-style components in logseq_journal).New node kinds (
schema/components.json→ regenerated all wire schemas):file-image— leaf. Required proppath(absolute path orfile://URL); optionalmax-pixel-size(int > 0) plus the standard sizing (width/height/max-width/max-height/corner-radius) andpress-enabled+Press. Apple backend decodes off the main actor viaCGImageSourceCreateThumbnailAtIndexwith anNSCachekeyed by"<maxPixelSize>|<path>"(~32MB cost cap, cost = bytesPerRow × height, default maxPixelSize 1024), shows aProgressViewwhile loading and a fallback icon on failure.file-preview— non-rendering leaf that presents the file via QuickLook. Design choice: a separate node kind rather than apreview-pathprop onfile-image, matching the sheet/dialog "mount to present" pattern: presence in the tree means presented; dropping the node dismisses; interactive close emitsDismiss. It deliberately does not joinisModalSurface— that invariant would force atexttitle and sheet chrome — instead it has its own presentation lane (LUIFilePreviewStore+.quickLookPreview) synced by the samesyncModalPresentationtree walk, with the same pending-dismissal suppression as sheets.file-previewis a restrictive kind: onlypath+accessibility-identifier.link— container. Requiredurl(string); renders SwiftUILink(destination:). Children form the label, ortext/icon/icon-placementprops when childless — so it composes inside list rows and overlays.New properties:
path,url,max-pixel-size. New wire kinds:"file-image","file-preview","link"(container: true). Events:Presson file-image,Dismisson file-preview — no new event shapes.Every journal-media item kind maps cleanly:
file→ file-image + file-preview,external→ link,placeholder→ existing skeleton/icon/spinner, menus → existing dropdown-menu/menu-item.Other backends: flutter/qt/winui/web schema mirrors updated (support checks, child rules, property validation); Qt gains stub QML views registered in
qt_add_qml_module; WinUI's generatedRestrictiveMatrixcovers file-preview automatically. No real rendering on non-Apple platforms per scope.Tests: OCaml
test_lui.mlgains property/event matrix + mount-op assertions; Swift gainsmapsLink,mapsFileImage,mapsFilePreview(store sync, dismiss suppression, restrictive-kind rejection).dune build,dune runtest -j 4(31 tests),make test-schema,swift test(136 tests) all green locally; WinUIdotnet testruns on CI.Link to Devin session: https://app.devin.ai/sessions/1867341ba00d443e9cb294081562ccde
Open in Devin Desktop: https://app.devin.ai/desktop/session/1867341ba00d443e9cb294081562ccde?variant=devin
Requested by: @RCmerci