[4.0] Select image/audio/video/document file from Media Manager #34634
Conversation
joomla-cms-bot
added
Language Change
|
looking forward to testing this tomorrow |
|
Tested using your supplied files. Only the pdf uploads. The mp3 and mp4 give a generic cant be uploaded message. |
|
The problem was in your test instructions. Better to clear the existing values in the options and then save in order to get the defaults. If it had been customised at all it would fail |
|
Click on a pdf file and then click on the three dots |
|
mp4 files have previews but there is no preview icon (you need to double click on the file to see the preview) pdf files do not have previews but you can still double click on the file and the previous item is displayed |
|
If you dont check the box "Use native elements" before inserting then nothing added to the article |
|
For files and folder deletion on update with script.php you can leave it to @wilsonge and me. One of us will run the tool to generate the lists of files and folders when this PR has been merged and before the next release after that. So it does not need to be done in this PR. But if you want to do it here because you wanna make it right, I can help later. For SQL scripts we have to decide how much we want to touch the media manager options on update. Since it will be at least risky and hacky trying to add single values to the json, we might have to completely replace the params so people would lose their settings on update if they have changed some. I can help with making the update SQL but not with |
|
@richard67 whichever is decided on the sql there should be a postinstallation message. either saying the settings have been reset to the defaults or saying that you can update the settings to add support for pdf, etc |
Yeah, that makes sense. Personally I'd prefer the "We don't touch the settings and let the postinstall message tell people that they can update their settings to add support for PDF and so on" approach. It can be that we have already an update SQL touching these parameters when updating from 3.10. If this is the case, we could modify that one to set them right. I have to check if there is one. |
|
IIRC we discussed this when I added webp support (or tried to) |
|
I've just checked: It seems there is no update SQL script where we already touch the params of the com_media extension. So there is no need to modify such a script. |
Fixed, there are 3 modes: embed, insert asa link with predefined text, insert as a link with selected text from the editor (the 2nd was missing)
Some side effect from copy/pasting,
Will update the instructions for existing installations A comment about SQL files and B/C:
|
@dgrammatiko It would be good to have that post install script or procedure. But that would maybe be better a new routine in the script.php than an update SQL script. If necessary @joomdonation could have a look on it. |
dgrammatiko
force-pushed
the
—media-select-v2—4.0-dev
branch
from
998a3c2 to
4fb968b
Compare
|
Preview of video now working correctly Preview of pdf - almost - the preview opens but its black. Clicking on rotate makes it appear (maybe that will give a clue) Audio file will not upload |
Make sure the legal mime types are: |
|
Checked that twice before posting
|
|
Article intro and full image |
Yes, I couldn't decide what would be the best scenario for this case so I silently ignored anything non-image. This might need an alert or something similar.
There's something fishy as the tests are also failing for no obvious reason |
|
@dgrammatiko Here the screenshot from drone from system tests: https://ci.joomla.org/artifacts/joomla/joomla-cms/4.0-dev/34634/system-tests/45401/MediaListCest.showsDefaultFilesAndFolders.mysql.fail.png It says "syntax error, unexpected ')'". Here the part of the log with the test where it fails: Does that help? |
Yup, turns out PHP is far away from becoming a functional programming language... |
|
This seems to important to not merge right now given it has b/c implications. Although it's not great given our point of RC. Thankyou so much @dgrammatiko for your hard work on this! |
|
woohoo!!! |
|
Thanks @wilsonge the B/C break is the change of some of the com_media params and to be honest here even what I did here is not perfect. An ideal solution is to have a data structure like {
supported: [
{
extension: 'png',
mine: 'image/png',
preview: true,
upload: true
}
]
}That will allow to remove the hardcoded preview extensions in the com_media app (vue) and also if done right (subforms?) it will be easier in the backend (eg have a pool that users enable/disable, instead of typing tings) |
| JFIELD_MEDIA_WIDTH_LABEL="Width" | ||
| JFIELD_MEDIA_TITLE_LABEL="Title" | ||
| ; Do not translate the text between the {} | ||
| JFIELD_MEDIA_UNSUPPORTED="You don't have a {extension} plugin, but you can {tag} download the {extension} file.</a>" |
There was a problem hiding this comment.
I see that it is combined. But is everything true in this line? Confused by the end of the line.
There was a problem hiding this comment.
Hmm, yes ... where does that closing anchor tag belong to?
There was a problem hiding this comment.
There is also the same ending in another line. Copy / paste
There was a problem hiding this comment.
@Kostelano Maybe you can make an issue, or if you have more time make a PR to fix it? It would need to get that text shown and then check the original markup. Browser inspection tools might repair the invalid markup and so not show any mistake, so it could be tricky.
There was a problem hiding this comment.
The first part is here
There was a problem hiding this comment.
I see ... not nice, but ok ... thanks for reporting back.
There was a problem hiding this comment.
I just realized that the closing tag shouldn't be in the ini file. Well, my bad 😞
There was a problem hiding this comment.
You can make a PR ... but it should be soon because there might be language freeze for J4 in near future.
| <div class="media-browser-item-preview"> | ||
| <div class="file-background"> | ||
| <div class="file-icon"> | ||
| <span class="fas fa-file-pdf" /> |
There was a problem hiding this comment.
Can you please improve this to reflect their respective icons instead of a PDF icon for all document types? Thanks.
There was a problem hiding this comment.
What would be the appropriate icon? Also you can PR the change yourself nothing too fancy here 😉
There was a problem hiding this comment.
pdf icon for .pdf
word icon for .doc
excel icon for .xls
otherwise default to generic file icon for the other extensions
There was a problem hiding this comment.
Don’t forget docx, docm, xlsx and xlsm
There was a problem hiding this comment.
You can do this in pure css so that the markup doesnt have to contain everything. See this example https://gschoppe.com/uncategorized/add-auto-sensing-file-type-icons-to-lists-of-downloads-with-fontawesome-and-css/
There was a problem hiding this comment.
So change line 10 to <span v-bind:class="getFileClass" />
and add after line 44 (after the data(){} ):
computed: {
/* Get the hashed URL */
getFileClass: (item) => {
switch (item.mime_type) {
case 'application/pdf':
return 'fas fa-file-pdf';
case 'application/msword':
case 'application/vnd.openxmlformats-officedocument.wordprocessingml.document':
return 'fas fa-file-word';
case 'application/vnd.ms-excel':
case 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet':
return 'fas fa-file-excel';
case 'application/vnd.ms-powerpoint':
case 'application/vnd.openxmlformats-officedocument.presentationml.presentation':
return 'fas fa-file-powerpoint';
default:
return 'fas fa-file';
}
},
},Modify it to your taste
There was a problem hiding this comment.
You can do this in pure css so that the markup doesnt have to contain everything.
Actually, on the web, the extensions are totally FOOBAR. The only way to tell what a type of a file really is, is by checking the mime type. The mime type is not available to the HTML/CSS unless the application actually passes it either as a class or a data attribute.
What I'm saying is that you can have a totally valid PDF (or whatever) without any extension. Actually, Joomla should never ever do anything based on the extensions as this is a huge SECURITY risk.
/rant
There was a problem hiding this comment.
and add after line 44 (after the data(){} ):
Should it be after line 177 in the same file?
There was a problem hiding this comment.
simple pr to correct an error from joomla#34634 where png was added twice
* [4.3] Remove duplicate png simple pr to correct an error from #34634 where png was added twice * Update base.sql * Update base.sql
Pull Request for Issue #32642 and https://www.facebook.com/groups/joomlanospam/posts/10157777796480997/?__cft__[0]=AZXC8G44TJLPO0QeEbytZwSVaZX2YpNFHjOUcx1xUqyJjBUZGqgh763LWT-Lx2xKyvJfIm-LQO6wUquCrqneOLqSAlx3AOpRDAxnMrliJHTTK1AIkT4Mh-BVfJCJjMVFEyDYelefuWE8UMA3X6_-XUFRasmSPT8VNhUVlOln0Ujj4LCrwnBYH_QLfColkn2QEkk&__tn__=%2CO%2CP-R
Apparently, the Joomla community finds it totally fine to use software that is based on a javascript that's 2 major versions behind. It's not fine for me thus the PR...
Summary of Changes
Please extract this
Archive.zip
to the images directory, or use your own mp3/mp4/pdf files
Testing Instructions
Apply the PR or downlad the installable from github
If you're applying the PR then go to the Media manager options and
Make sure the legal mime types are:
image/jpeg,image/gif,image/png,image/bmp,audio/mp3,video/mp4,application/msword,application/excel,application/pdf,application/powerpoint,text/plain,application/x-zipthe video extensions are
mp4the audio extensions are
mp3the document extensions are
odg,odp,ods,odt,pdf,png,ppt,txt,xcf,xls,csvand finally the images extensions are:
bmp,gif,jpg,jpeg,png,icoThis is IMPORTANTGo to tinyMCE and:
Select the image from the CMS Content dropdown
Then select the video file, click on the controls and embed checkboxes, change the width and height and then click select image at the bottom of the modal
Press couple returns type
Video Linkand select the text you just wroteThen select the video file, without selecting the controls and embed checkboxes (width and height is irrelevant here) and then click select image at the bottom of the modal
Check the produced HTML by clicking the toggle button under the editor
Select the image from the CMS Content dropdown
Then select the audio file, click on the controls and embed checkboxes and then click select image at the bottom of the modal
Press couple returns type
Audio Linkand select the text you just wroteThen select the audio file, without selecting the controls and embed checkboxes and then click select image at the bottom of the modal
Check the produced HTML by clicking the toggle button under the editor
Select the image from the CMS Content dropdown
Then select the PDF file, click on the embed checkboxe change the width and height and then click select image at the bottom of the modal
Press couple returns type
PDF Linkand select the text you just wroteThen select the PDF file, without selecting the embed checkboxe and then click select image at the bottom of the modal
Check the produced HTML by clicking the toggle button under the editor
your final html should look like:
Check that the insertion of an image is still ok
Check that the media FIELDS (intro/fulltext images) are still functional
Finally save the article and visit it in the front end.
That's it.Edit the definition of the intro image
joomla-cms/administrator/components/com_content/forms/article.xml
Lines 735 to 739 in 7f5dd8e
types="images,audios"Test the field, you should be able now to select images and audio files, also try to upload an unsupported file (eg video and pdf)
Repeat with different combinations
types="images,audios,videos,documents"Actual result BEFORE applying this Pull Request
Impossible to select/embed anything other than an image file
Expected result AFTER applying this Pull Request
It's ok to select/embed audio, video, document files
Front end:
Documentation Changes Required
There are few todos that I skipped here:
@wilsonge @laoneo any feedback?