Skip to content

[RFC] feat(viewer)!: tell openers with events instead of callbacks - #130

Merged
skjnldsv merged 1 commit into
mainfrom
feat/viewer-events
Oct 8, 2026
Merged

skjnldsv merged 1 commit into
mainfrom
feat/viewer-events

Conversation

@skjnldsv

@skjnldsv skjnldsv commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Handlers already talk to the viewer with events on their element (loaded, errored, update:*). Openers were the odd one out, with onPrev, onNext, onClose and onEditingChange callbacks in the open options. The viewer app's downloadCallback, which Text used to save unsaved edits before a download, had no equivalent at all. This moves all of it to events, for discussion before 2.0.0 is stable.

open(), openFolder() and compare() resolve with a session, an EventTarget dispatching:

Event detail Replaces
update:file [file] onPrev, onNext
update:editing [editing] onEditingChange
close [] onClose
before-download { file, waitUntil } the viewer app's downloadCallback

before-download also goes to the handler's element, which is where Text's editor and its unsaved state live (nextcloud/text#9349). getViewer() dispatches all four for any page, whoever opened the viewer. The names and detail shapes match what handler elements emit.

I kept what the callbacks did across openings. The file shown and editing only reach the latest opening, the way onPrev/onNext came from the current options. close reaches every opening since the viewer opened, the way every onClose was kept. The Files history follows only the latest opening it made: the Files app opens the same file again as the sidebar opens, and unwinding the history once per opening ran history.go() twice. There's a test for that now.

loadMore, enabled, preload and onInit stay functions, since the viewer needs their answer.

This includes #129, which can be closed if this goes in first. It breaks the server's OCA.Viewer shim (apps/viewer/src/legacy.ts), which maps onPrev, onNext and onClose to these options and is what Photos goes through. That needs porting with the bump. Text, Whiteboard and the server's other callers don't pass these options. I dropped the test "tells the others when one of them throws", because dispatchEvent already keeps a throwing listener from stopping the others.

Unit and Playwright tests pass locally.

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

@skjnldsv skjnldsv added this to the 2.0.0 milestone Oct 8, 2026
@skjnldsv skjnldsv added status: developing Work in progress type: enhancement 🚀 New feature or request AI assisted labels Oct 8, 2026
@skjnldsv skjnldsv self-assigned this Oct 8, 2026
@codecov

codecov Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.98658% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.97%. Comparing base (789469f) to head (c446c33).

Files with missing lines Patch % Lines
lib/views/Viewer.vue 95.45% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #130      +/-   ##
==========================================
+ Coverage   92.82%   92.97%   +0.15%     
==========================================
  Files          47       49       +2     
  Lines        4069     4172     +103     
  Branches      813      816       +3     
==========================================
+ Hits         3777     3879     +102     
- Misses        273      274       +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.

@skjnldsv
skjnldsv marked this pull request as ready for review October 8, 2026 09:00
@skjnldsv skjnldsv changed the title feat(viewer)!: tell openers with events instead of callbacks [RFC] feat(viewer)!: tell openers with events instead of callbacks Oct 8, 2026
@skjnldsv
skjnldsv force-pushed the feat/viewer-events branch from 4e76ec8 to 9a5cd8d Compare October 8, 2026 09:12
@skjnldsv
skjnldsv changed the base branch from main to feat/before-download October 8, 2026 09:12
@skjnldsv
skjnldsv force-pushed the feat/viewer-events branch 2 times, most recently from b3c36da to 4e76ec8 Compare October 8, 2026 12:23
@skjnldsv
skjnldsv changed the base branch from feat/before-download to main October 8, 2026 12:23
Handlers tell the viewer with events on their element, and the viewer
tells them with props. Its openers were the odd one out, with onPrev,
onNext, onClose and onEditingChange callbacks in the open options, and the
viewer app's downloadCallback, which Text used to save edits before a
download, had no equivalent at all.

open(), openFolder() and compare() now resolve with a session, an
EventTarget that dispatches `update:file`, `update:editing` and `close`,
shaped like what handler elements emit. The file shown and editing reach
the latest opening only, closing every opening since the viewer opened, as
the callbacks did. The viewer itself (getViewer()) dispatches the same
events for any page.

Before its own Download, Ctrl+S and the Files download action, the viewer
dispatches `before-download` on the handler's element, the session and the
viewer. What they hand `waitUntil()` holds the download, and a promise that
rejects cancels it, with an error.

The Files history follows the latest opening it made only: the Files app
opens the same file again as the sidebar opens, and both openings hear the
viewer close.

BREAKING CHANGE: the onPrev, onNext, onClose and onEditingChange options
are gone; listen to the session open() resolves with.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: John Molakvoæ <14975046+skjnldsv@users.noreply.github.com>
@skjnldsv
skjnldsv force-pushed the feat/viewer-events branch from 4e76ec8 to c446c33 Compare October 8, 2026 12:25

@mejo- mejo- left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code changes look good, and indeed much cleaner interface than before.

@skjnldsv
skjnldsv merged commit 059deb4 into main Oct 8, 2026
21 checks passed
@skjnldsv
skjnldsv deleted the feat/viewer-events branch October 8, 2026 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI assisted status: developing Work in progress type: enhancement 🚀 New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants