Skip to content

feat(viewer): let a handler finish with a file before it is downloaded - #129

Closed
skjnldsv wants to merge 2 commits into
mainfrom
feat/before-download
Closed

skjnldsv wants to merge 2 commits into
mainfrom
feat/before-download

Conversation

@skjnldsv

@skjnldsv skjnldsv commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

The viewer app awaited a handler's downloadCallback before downloading the file shown, and Text used it to save edits not written yet. This library has no equivalent, so on 36 a download from the viewer gets the last saved version of a document.

The viewer now dispatches before-download on the handler's element before its own Download, Ctrl+S and the Files download action. The element hands detail.waitUntil() a promise; the download waits for it, and is cancelled with an error if it rejects. I put it on the element rather than on the handler registration, since the editor and its unsaved state live there: Text no longer has to find its editor through window.OCA.Text.editorComponents.

Component tests cover each of the three paths waiting, and a rejection cancelling with a message. They fail on main. The Text side is in nextcloud/text#9349

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

The viewer app called a handler's downloadCallback before downloading the
file shown, and Text used it to save edits not written yet. Without it, a
download from the viewer gets the last saved version of a document.

The viewer now dispatches `before-download` on the handler's element
before its own Download, Ctrl+S and the Files download action. The element
hands `waitUntil()` what the download should wait for; a promise that
rejects cancels it, with an error. On the element rather than the handler,
as that is where the editor and its unsaved state live.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: John Molakvoæ <14975046+skjnldsv@users.noreply.github.com>
@codecov

codecov Bot commented Oct 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.95652% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.79%. Comparing base (789469f) to head (9987482).

Files with missing lines Patch % Lines
lib/utils/beforeDownload.ts 88.46% 3 Missing ⚠️
lib/views/Viewer.vue 85.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #129      +/-   ##
==========================================
- Coverage   92.82%   92.79%   -0.03%     
==========================================
  Files          47       48       +1     
  Lines        4069     4110      +41     
  Branches      813      820       +7     
==========================================
+ Hits         3777     3814      +37     
- Misses        273      277       +4     
  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.

@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.

One nitpick comment. Otherwise looks good to me.

Comment thread lib/utils/beforeDownload.ts Outdated
Co-authored-by: Jonas <jonas@freesources.org>
Signed-off-by: John Molakvoæ <skjnldsv@users.noreply.github.com>
@skjnldsv
skjnldsv enabled auto-merge October 9, 2026 06:56
@skjnldsv

skjnldsv commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Closing: this went in with #130, which also sends before-download to the opener's session and to getViewer().

@skjnldsv skjnldsv closed this Oct 9, 2026
auto-merge was automatically disabled October 9, 2026 07:27

Pull request was closed

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

Labels

3. to review AI assisted type: bug 🐛 Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants