basecamp / basecamp/lexxy

Allow previewable PDFs in image galleries

Open
#1,237 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
1.2k
Forks
119
Avg merge
4d 8h
Merged PRs (30d)
15

Description

Galleries currently accept raster images only:

```js
// src/nodes/image_gallery_node.js
static isValidChild(node) {
return $isActionTextAttachmentNode(node) && node.isPreviewableImage
}

// src/helpers/html_helper.js
function isPreviewableImage(contentType) {
return contentType.startsWith("image/") && !contentType.includes("svg")
}
```

Because that is a content-type test rather than a "does this have a preview" test, it excludes PDFs even when Active Storage has generated a thumbnail and Lexxy is already displaying it.

That feels inconsistent with how Lexxy treats a previewable PDF everywhere else. `ActionTextAttachmentNode` renders it down the previewable path with `previewSrc` and `pendingPreview`, `BlobWithPreviewUrl` supplies a `resize_to_limit: [1024, 768]` preview URL, and the figure is given `attachment--preview`. A gallery is a layout container, and a PDF that already has a thumbnail lays out much like an image does.

Concretely: three scanned pages attached as PDFs cannot be arranged as a three-up gallery, while the same three pages exported as PNGs can.

## The obvious change, and why it may not be enough

The accessor already exists:

```js
get isPreviewableAttachment() {
return this.isPreviewableImage || this.previewable
}
```

Swapping `isValidChild` to use it would admit previewable PDFs while still excluding plain file attachments, which have no thumbnail and would not lay out sensibly. But it raises a few questions I do not think have obvious answers:

- **Videos come along for the ride.** `isPreviewableAttachment` is `isPreviewableImage || previewable`, so a `` with controls would become gallery-eligible too. That may be desirable, or may want its own predicate.
- **Aspect ratio.** `.attachment-gallery .attachment` is a third-width cell. An A4 page at roughly 3:4 in a third of the post width ends up small and very tall beside landscape photos. Galleries may need a max height, or a per-item aspect hint.
- **Captions.** Host apps render their own `active_storage/blobs/_blob` partial and receive `in_gallery: true`. Anything that puts a filename, size or download link under a standalone PDF would need to suppress it inside a gallery, or every cell gains a line of metadata.
- **Consistency across entry points.** Upload grouping and `#pastedImageFilesFrom` count images to decide whether to form a gallery, so those would want the same predicate. Otherwise uploading three PDFs at once behaves differently from dragging three together.

Happy to have a go at a PR if there is an appetite for it and you have a view on the video question and the aspect-ratio handling. If you would rather galleries stayed images-only, that is a reasonable answer too, in which case the drop affordance is the thing worth tightening: see #1235, where the drop markers appear over targets that cannot accept the drop.

## Version

lexxy 0.9.29, Rails 8.2.0.alpha, Active Storage with the Poppler previewer.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with src/nodes/image_gallery_node.js and src/helpers/html_helper.js, then trace ActionTextAttachmentNode, BlobWithPreviewUrl, and the upload-grouping and #pastedImageFilesFrom entry points. Review the .attachment-gallery styling and host active_storage/blobs/_blob partial behavior. Done requires a settled policy for PDFs, videos, aspect ratios, captions, and consistent gallery formation across entry points.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, rails
Domain
frontend, web-dev
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.