-
Notifications
You must be signed in to change notification settings - Fork 116
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
fix: Show PDF even when download hidden on public share #3872
Conversation
Signed-off-by: Elizabeth Danzberger <[email protected]>
Signed-off-by: Elizabeth Danzberger <[email protected]>
Signed-off-by: Elizabeth Danzberger <[email protected]>
/backport to stable29 |
/backport to stable28 |
/backport to stable27 |
src/public.js
Outdated
OC.Plugins.register('OCA.Files.NewFileMenu', NewFileMenu) | ||
} | ||
|
||
if (isPdf()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should only trigger this if it is actually "download hidden share" or if the files_pdfviewer app is disabled. Otherwise the regular pdf viewer should be preferred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm understanding you correctly, it should be fixed in my recent commits -- essentially, if the download is hidden or the files_pdfviewer is not enabled, open the PDF in richdocuments. Also open in richdocuments if it's an office document. otherwise, let the viewer handle it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, just went through testing the different constellations and looks all good 👍
Signed-off-by: Elizabeth Danzberger <[email protected]>
Signed-off-by: Elizabeth Danzberger <[email protected]>
Summary
Issue seemed to be caused by loading
files.js
in addition topublic.js
, so the logic was just moved to the newer implementation inpublic.js
.TODO
Checklist