Conversation
skjnldsv
requested review from
max-nextcloud,
mejo- and
silverkszlo
as code owners
September 22, 2026 18:25
skjnldsv
force-pushed
the
feature/migrate-new-viewer-api
branch
2 times, most recently
from
September 23, 2026 07:50
8f4b8b4 to
a6ddb27
Compare
Replace the legacy OCA.Viewer.registerHandler() global with registerHandler() from @nextcloud/viewer, following the removal of the bundled viewer app in favour of the package (nextcloud/server#63954). Key changes: - viewer.js: register a custom element (oca-text-viewer) via defineCustomElement and call registerHandler() from @nextcloud/viewer - TextViewerWrapper.vue: thin wrapper mapping new IFile-based ViewerProps to the existing ViewerComponent props - LoadViewerListener: switch from OCA\Viewer\Event\LoadViewer (gone with the viewer app) to OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent, loading text-viewer as an init script on every non-error page - Application.php: update event listener registration accordingly - Add @nextcloud/viewer ^2.0.0-beta.11 dependency canCompare and downloadCallback are not part of the new API and have been dropped. Assisted-by: ClaudeCode:claude-sonnet-4-6 Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
skjnldsv
force-pushed
the
feature/migrate-new-viewer-api
branch
from
September 23, 2026 07:57
a6ddb27 to
06f59d2
Compare
11 tasks
max-nextcloud
left a comment
Collaborator
There was a problem hiding this comment.
Thanks a lot for looking into this! Appreciate it a lot! ❤️
Just a few initial comments. Did not test nor look into test failures yet.
Comment on lines
+9
to
+10
| import { computed } from 'vue' | ||
| import ViewerComponent from '../components/ViewerComponent.vue' |
Collaborator
There was a problem hiding this comment.
Could we maybe make this an async component so it only loads when a text file is actually opened?
If I understand the code correctly TextViewerWrapper will be loaded asynchronously - but it will always be loaded, right?
Something like this maybe:
Suggested change
| import { computed } from 'vue' | |
| import ViewerComponent from '../components/ViewerComponent.vue' | |
| import { computed, defineAsyncComponent } from 'vue' | |
| const ViewerComponent = defineAsyncComponent(() => | |
| import('../components/ViewerComponent.vue') | |
| ) |
|
|
||
| public function handle(Event $event): void { | ||
| if (!$event instanceof LoadViewer) { | ||
| if (!($event instanceof BeforeTemplateRenderedEvent)) { |
Collaborator
There was a problem hiding this comment.
So we will load this for basically all templates now, right - even if no viewer is loaded at all. So we need to make sure this is small.
| $this->eventDispatcher->dispatchTyped(new RenderReferenceEvent()); | ||
|
|
||
| $this->initialStateProvider->provideState(); | ||
| if ($event->getResponse()->getRenderAs() === TemplateResponse::RENDER_AS_ERROR) { |
Collaborator
There was a problem hiding this comment.
Maybe exclude blank templates as well?
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Summary
Migrates the text viewer handler registration from the legacy
OCA.Viewer.registerHandler()global toregisterHandler()from@nextcloud/viewer, following the removal of the bundled viewer app in favour of the@nextcloud/viewerpackage.What changed:
src/viewer.js: registersoca-text-vieweras a custom element (via Vue'sdefineCustomElementwithshadowRoot: false) and callsregisterHandler()from@nextcloud/viewerinstead ofOCA.Viewer.registerHandler()src/views/TextViewerWrapper.vue: new thin wrapper that maps the newIFile-basedViewerProps(file, files, editing, …) to the existingViewerComponentprops (filename, fileid, mime)lib/Listeners/LoadViewerListener.php: rewritten to listen onOCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent(fires on every non-error page) instead of the now-goneOCA\Viewer\Event\LoadViewer; loadstext-vieweras an init scriptlib/AppInfo/Application.php: updated event listener registration accordingly; drops the deadOCA\Viewerimport@nextcloud/viewer ^2.0.0-beta.11dependencycanCompareanddownloadCallbackare not part of the newIHandlerAPI and have been dropped.🖼️ Screenshots
No UI change: the viewer behaviour is unchanged.
🏁 Checklist
npm run lint/npm run stylelint/composer run cs:check)🤖 AI (if applicable)