Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add preview provider for emf files #3303

Merged
merged 2 commits into from
Nov 20, 2023
Merged

feat: add preview provider for emf files #3303

merged 2 commits into from
Nov 20, 2023

Conversation

kesselb
Copy link
Contributor

@kesselb kesselb commented Nov 17, 2023

  • Target version: main

Summary

For nextcloud/files_emfviewer#1
In addition to nextcloud/server#41395

We added support for image/emf via LibreOffice for 28.

For setups already using Richdocuments/Collabora it's nicer to let the previews generate via collabora web and no local LibreOffice installation is required.

TODO

  • CI
  • Test
  • Documentation

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Documentation (manuals or wiki) has been updated or is not required

@@ -88,6 +89,7 @@ public function register(IRegistrationContext $context): void {
'getPathForToken',
'getWopiForToken',
]);
$context->registerPreviewProvider(EMF::class, '/image\/emf/');
Copy link
Member

Choose a reason for hiding this comment

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

Did you give that a test with a Collabora intance setup? We have not migrated our preview class to the IProviderV2 interface (which we really should 🙈 ) and as far as I see it the context registration requires that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you give that a test with a Collabora intance setup?

Yep, it works like a charm ;)

the context registration requires that.

True, the psalm-param type hints IProviderV2.

I guess it's better to use the deprecated way that expects a IProvider instance for now.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, let's stick to that then for now

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@kesselb kesselb merged commit 37d67ff into main Nov 20, 2023
57 checks passed
@kesselb kesselb deleted the preview-emf branch November 20, 2023 15:56
@kesselb
Copy link
Contributor Author

kesselb commented Nov 20, 2023

Documentation: nextcloud/documentation#11292

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Ready to be reviewed enhancement New feature or request feature: previews and thumbnails
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants