knowledge
knowledge copied to clipboard
[16.0][MIG] attachment_preview: Migration to 16.0
I squashed a few commits according to https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests
This PR has 2 commits: 1 with the actual migration; 1 where I move code around without changing anything. I can squash them if you feel it's better
I made sure this plays well with web_responsive
& especially web_chatter_position
with its 3 chatter settings (responsive / bottom / sided)
I added a ROADMAP file with possible future improvements
Overall implementation remains the same as in previous versions:
- patch into attachment list to prepare prev/next buttons in the viewer
- patch into attachment cards to observe clicks on new buttons this module adds
- fetch attachment extensions
- add an iframe into the DOM tree next to the main form one, display viewer inside it
- add preview button within binary fields
Main changes in this migration:
- update imports/exports to proper JS modules - in particular, this fixes "service already defined" console messages we also get in 15.0
- rework FormRenderer injector, previous (legacy) one was no longer used
- patch attachment list / cards with new methods in the mail module
- fix preview button inclusion within binary fields
- fix previous/next viewer buttons
Screenshots from runboat
2 buttons in attachment cards to preview inline & open in new tab:
Inline preview of a PDF:
Inline preview of an ODT (which I navigated to using left arrow at the top of the viewer):
Binary field with button to open in new tab:
Hi this PR is ready, please review
@vancouver29 @HolgerNahrstaedt @holgern I am tagging you as you worked on https://github.com/OCA/knowledge/pull/363 (15.0 mig)
@hbrunn I am tagging you as you bootstrapped this module, in case you're still around
Thank you for migrating this!
Despite having no browser or server error, the preview doesn't seem to work in either FF or Chrome for anything but pdf files. With other types that should be supported by viewerJS (doc, docx, ...), the preview is just empty, and opening in a new tab it spawns a "not supported filetype" popup. I've tested on two device with the same results. Do you have any idea about what could be wrong?
Thank you for migrating this!
Despite having no browser or server error, the preview doesn't seem to work in either FF or Chrome for anything but pdf files. With other types that should be supported by viewerJS (doc, docx, ...), the preview is just empty, and opening in a new tab it spawns a "not supported filetype" popup. I've tested on two device with the same results. Do you have any idea about what could be wrong?
Hi @len-foss did you test on runboat? My screenshots in this issue are from runboat; the file with "character formatting" / "paragraph formatting" is an ODT file.
I'm using Chromium mainly, but I just tried with Firefox in an incognito window; no problem either.
At the moment, the only supported file extensions are those in attachment_preview/static/src/js/utils.esm.js
; doc & docx are not included there.
In this PR I did not update the viewerjs lib; we could update it in a separate PR and add new supported filetypes in there.
@houzefa-abba I tried in combination with web_responsive
, there is a glitch:
@houzefa-abba oh thanks, indeed it works fine with odt files, runboat or local. I agree doing the update in a separate PR is the proper way to do it, let's just hope it gets through quickly for once.
I confirm the behavior reported by @astirpe, although I wouldn't call it a glitch. It seems you could trivially fix it by setting the z-index of attachment_preview_widget
to 199 instead of 999 -- the full-screen-dropdown
has a z-index of 200, so that would go below. The 200 value is also used by the pdf viewer so it seems to confirm that idea.
@houzefa-abba I tried in combination with
web_responsive
, there is a glitch:
Good catch! Fixed with z-index change suggested by @len-foss. I went with z-index=100 as most Odoo elements are between 1-10 and other ones such as pop-ups are >=200.
I also used some if (x) return;
1-liners which I noticed in your code 🙃
I added widget clean-up in my last change, otherwise I was getting multiple widgets in the DOM tree when navigating in and out of form views
/ocabot migration attachment_preview
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.