Skip to content

Enable editable captions for video attachments#950

Merged
jorgemanrubia merged 1 commit into
mainfrom
fix-video-caption
Apr 9, 2026
Merged

Enable editable captions for video attachments#950
jorgemanrubia merged 1 commit into
mainfrom
fix-video-caption

Conversation

@jorgemanrubia
Copy link
Copy Markdown
Member

Summary

  • Video attachments now have an editable caption textarea, matching the behavior of image attachments
  • Previously, videos were rendered with a static file-style caption (filename + size only), making it impossible for users to add a custom caption
  • Adds an isVideo getter to ActionTextAttachmentNode and a dedicated branch in createDOM() that renders the file icon with an editable caption

Basecamp card

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Enables editable captions for video attachments so users can add custom captions, aligning video attachment UX more closely with image attachments.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Changes:

  • Add isVideo to ActionTextAttachmentNode and render videos with a file icon + editable caption textarea.
  • Add Playwright coverage to verify video captions render as editable and persist to serialized editor output.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/nodes/action_text_attachment_node.js Adds video-specific DOM rendering path and isVideo helper.
test/browser/tests/attachments/video_caption.test.js Adds browser tests to ensure video captions are editable and saved.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

figure.appendChild(this.#createEditableCaption())
} else if (this.isVideo) {
figure.appendChild(this.#createDOMForFile())
figure.appendChild(this.#createEditableCaption())
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Video attachments still render with attachment--file styling but now use the editable caption textarea. In the editor stylesheet, .attachment__caption textarea forces text-align: center (app/assets/stylesheets/lexxy-editor.css:349-365), while file attachments are styled to be start-aligned via .attachment--file .attachment__caption { text-align: start; } (app/assets/stylesheets/lexxy-content.css:353-365). This will make video captions appear centered in a file-style row layout, which is inconsistent with other file attachments. Consider adding a CSS override for .attachment--file .attachment__caption textarea (or otherwise inheriting alignment) so video captions follow the file attachment alignment.

Suggested change
figure.appendChild(this.#createEditableCaption())
const caption = this.#createEditableCaption()
const textarea = caption.querySelector("textarea")
if (textarea) textarea.style.textAlign = "start"
figure.appendChild(caption)

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The centered text-align on the caption textarea is intentional editor styling — it matches image attachment captions and provides a consistent editing experience. The .attachment--file .attachment__caption { text-align: start } in lexxy-content.css targets the rendered (non-editor) view where the caption is plain text, not a textarea. Adding an inline style override here would fight against the editor stylesheet and create a special case for video. If the alignment needs adjusting, it should be done in the CSS layer, not in the DOM-building logic.

@jorgemanrubia jorgemanrubia added the fix-validated Bug fix manually validated: broken in production, fixed locally label Apr 8, 2026
@jorgemanrubia
Copy link
Copy Markdown
Member Author

Manual Validation: Validated

Injected a <action-text-attachment> with content-type="video/mp4" into the editor and verified caption editability.

Production (https://basecamp.github.io/lexxy/try-it.html): the video attachment rendered with a static caption showing only the filename ("clip.mp4") and file size ("96.45 KB") — no editable textarea was present. The figcaption contained <strong class="attachment__name"> and <span class="attachment__size"> elements only.

Local (built JS from fix-video-caption branch): the video attachment rendered with an editable <textarea> in the figcaption, matching the behavior of image attachments. Clicked the textarea, typed "My video caption", and confirmed the text was accepted. The figcaption contained <textarea placeholder="clip.mp4" rows="1">.

Video attachments were rendered with a static file-style caption (just the
filename and size), making it impossible to add a custom caption. Images
already got an editable textarea via the `isPreviewableAttachment` path,
but videos fell through to the non-editable `#createDOMForNotImage()` path.

Add an `isVideo` getter and a dedicated branch in `createDOM()` so video
attachments get the file icon alongside an editable caption textarea.
@jorgemanrubia jorgemanrubia marked this pull request as ready for review April 9, 2026 12:39
Copilot AI review requested due to automatic review settings April 9, 2026 12:39
@jorgemanrubia jorgemanrubia merged commit 65bc4ef into main Apr 9, 2026
12 of 13 checks passed
@jorgemanrubia jorgemanrubia deleted the fix-video-caption branch April 9, 2026 12:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix-validated Bug fix manually validated: broken in production, fixed locally

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants