Skip to content

feat(viewer): name the element that shows a file, to render it elsewhere - #97

Open
skjnldsv wants to merge 1 commit into
mainfrom
feat/embed-element
Open

skjnldsv wants to merge 1 commit into
mainfrom
feat/embed-element

Conversation

@skjnldsv

@skjnldsv skjnldsv commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

The preview of a file link (the Files reference widget, shown in Talk, Text, Collectives and the Smart Picker) used to render the file inline: the video playing, the PDF scrolling, right in the message. It found the handler's Vue component through OCA.Viewer.availableHandlers, which is gone, so the widget always falls back to its static card.

getViewer().elementFor(file) gives the tag name of the handler's custom element, once it's defined, or nothing when no handler takes the file or it can't be read. It reads the shared registry and loads nothing of the viewer itself. The new embedded prop on ViewerProps tells the handler it is shown inline: videos and sounds then wait to be played instead of starting on their own. It's a hint not to take over the page: editing is usually better left to the viewer, which the caller can open on the file.

The server widget gets ported to it once this is released, and keeps its own card as the fallback.

Unit tests cover the element being named without loading the viewer, onInit running first, nothing for a file no handler takes or that can't be read, and autoplay off when embedded. The README has a "Show a file outside the viewer" section.

👾 This pull request was assisted by Claude Code, commits carry an Assisted-by trailer.

@skjnldsv skjnldsv added AI assisted status: review Waiting for reviews type: enhancement 🚀 New feature or request labels Oct 2, 2026
@codecov

codecov Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.23529% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.39%. Comparing base (2c82dd9) to head (bc938cc).

Files with missing lines Patch % Lines
lib/viewer.ts 85.71% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #97      +/-   ##
==========================================
+ Coverage   91.33%   91.39%   +0.06%     
==========================================
  Files          41       41              
  Lines        3532     3546      +14     
  Branches      816      819       +3     
==========================================
+ Hits         3226     3241      +15     
+ Misses        287      286       -1     
  Partials       19       19              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread lib/viewer.ts Outdated
/**
* Whether the file is shown inline, outside the viewer, as the preview
* of a link to it (see `getViewer().elementFor()`). Handlers should then
* show it without taking over: no autoplay, read-only where editing is

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Current reference widgets allow switching to write mode and inline editing for some file types. At least Text (for Markdown/plaintext) and Richdocuments (for office documents) support it as far as I know. I'm not sure though whether we want to keep it as it often is very bad UX. Personally I'd prefer a button to open the previewed file full-screen in a overlay modal, read-write where supported.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I reworded it so embedded is only a hint and Richdocuments can still decide for itself. For the files widget I'd do what you suggest: read-only inline, plus a button to open it in the viewer.

The preview of a file link (the files reference widget, in Talk, Text and
the like) used to render the viewer handler's component inline, found
through OCA.Viewer.availableHandlers. Handlers are custom elements now,
and the registry is not something a page should dig through, so
getViewer().elementFor(file) names the element of the handler that would
open the file, defined and ready to render, without loading the viewer.

`embedded` on ViewerProps tells the handler it is shown inline, which the
old widget said with `isEmbedded` and `active`. A hint not to take over the
page: videos and sounds wait to be played, and editing is better left to
the viewer.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: John Molakvoæ <14975046+skjnldsv@users.noreply.github.com>
@skjnldsv
skjnldsv force-pushed the feat/embed-element branch from 7401f98 to bc938cc Compare October 7, 2026 08:41
@skjnldsv
skjnldsv enabled auto-merge October 7, 2026 09:10

This branch has not been deployed

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

Labels

AI assisted status: review Waiting for reviews type: enhancement 🚀 New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants