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

Add file types for media custom field plugin. #45013

Open
wants to merge 79 commits into
base: 6.0-dev
Choose a base branch
from

Conversation

sergeytolkachyov
Copy link
Contributor

@sergeytolkachyov sergeytolkachyov commented Feb 25, 2025

When creating a custom field, do you want to be able to select not only images, but also documents, videos, and audio? This PR adds the ability to specify one or more file types for a custom media type field.

Summary of Changes

  • The file types parameter was added to the field parameters during creation.
  • There are 4 file types: images, audios, videos, documents. This list decides which of the allowed file extensions from Media Manager configuration are used.
  • images file type is selected by default and for empty parameter value
  • You can select one or several file types: only videos and documents for example
  • new attribute for Accessiblemedia Form Field: types. Similar to Media field.

Testing Instructions

  • Apply this PR
  • Create a new custom field for articles or contacts.
  • Look at new field parameter - File types
    image
  • save your field with default settings (images type is selected)
    image
  • go to article edit view and try to choose a media file in this field - You'll be able to select only images. This is default Joomla behavior before apply this PR.
    image
    See also a modal window title is Change image
    image
    See also that the image has been rendered successfully in the frontend
    image
  • return to field settings and change file types. For example, unselect an images and select a documents or videos or both of them. Save field params,
    image
  • go to article edit view again and try to select media file in this field. Make sure you can select only documents or only videos as you configured your field. See also a modal window title is Change file
    image
    See that there is no alt text and empty alt fields, but new field link text is present
    image
  • Check that link to download selected file has been rendered successfully in the frontend for documents file types. The link text is download by default. You can specify your own text.
    image
  • Check that <video> tag for selected file has been rendered successfully in the frontend for video file types.
    image
  • Check that <audio> tag for selected file has been rendered successfully in the frontend for audio file types.
    image
  • Go to field settings and change file types and add to file types images. So you have both images and non-images file types selected. Save field params.
    image
  • Go to article edit view and check that alt text and empty alt fields are present with the link text field. So if you'll select an image file - you can use additional field for image. If you'll select a non-image file - you can use a link text field
    image
  • Check modal window title is Change file. Check that you can choose both images and non-images file types.
    image
    image
  • Select document file and check that link to download selected file has been rendered successfully in the frontend. Make sure that the link text matches the one specified in the field link text.
    image
  • Then select image file, fill the alt text for test and check that the image has been rendered successfully in the frontend
    image
    image

Actual result BEFORE applying this Pull Request

You cannot select anything except images in custom fields,

Expected result AFTER applying this Pull Request

Now you can configure file types for custom field type media (wich is a bundle of media + text for alt + checkbox for empty alt). You can select a mp4 or pdf in your media custom field.

  • If image file has been selected - you can use alt text and empty alt fields. Image will render in frontend.
  • if audio or video file has been selected - <audio> or <video> tag will render in frontend.
  • if document file has been selected - you can use link text field for download link. Download link will render in frontend.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

Sorry, something went wrong.

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-5.3-dev labels Feb 25, 2025
Co-authored-by: Quy Ton <quy@nomonkeybiz.com>
@Septdir
Copy link
Contributor

Septdir commented Feb 26, 2025

I have tested this item ✅ successfully on 9a36d9b


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45013.

1 similar comment
@gug2
Copy link

gug2 commented Feb 26, 2025

I have tested this item ✅ successfully on 9a36d9b


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45013.

@hans2103
Copy link
Contributor

I have tested this item ✅ successfully on 9a36d9b


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45013.

Co-authored-by: Brian Teeman <brian@teeman.net>
@sergeytolkachyov
Copy link
Contributor Author

I have tested this item ✅ successfully on dff217a


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45013.

1 similar comment
@hans2103
Copy link
Contributor

I have tested this item ✅ successfully on dff217a


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45013.

@sergeytolkachyov
Copy link
Contributor Author

@hans2103 can you test again, please?

@gug2
Copy link

gug2 commented Feb 26, 2025

I have tested this item ✅ successfully on dff217a


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45013.

1 similar comment
@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on dff217a


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45013.

Co-authored-by: Quy Ton <quy@nomonkeybiz.com>
@sergeytolkachyov
Copy link
Contributor Author

@gug2 @web-eau-net @hans2103 @viocassel can you test again?

@gug2
Copy link

gug2 commented Mar 3, 2025

I have tested this item ✅ successfully on 887919c


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45013.

@brianteeman
Copy link
Contributor

still get php warning on non-image media

example

Mar-2025 12:37:12 UTC] PHP Warning:  mime_content_type(images/rl.pdf): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99
[03-Mar-2025 12:37:12 UTC] PHP Warning:  mime_content_type(images/sampledata/output.mp4): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99

@sergeytolkachyov
Copy link
Contributor Author

still get php warning on non-image media

example

Mar-2025 12:37:12 UTC] PHP Warning:  mime_content_type(images/rl.pdf): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99
[03-Mar-2025 12:37:12 UTC] PHP Warning:  mime_content_type(images/sampledata/output.mp4): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99

I can't reproduce this issue both on Windows local server and remote Linux server. This is my result for pdf file.
image
image
This is for mp3
image

I came across with this issue only one time when the filename has contained a space. But this is not a problem of this PR, it is a MediaHelper problem that needs to be solved separately.

@sergeytolkachyov
Copy link
Contributor Author

still get php warning on non-image media

example

Mar-2025 12:37:12 UTC] PHP Warning:  mime_content_type(images/rl.pdf): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99
[03-Mar-2025 12:37:12 UTC] PHP Warning:  mime_content_type(images/sampledata/output.mp4): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99

Can you reproduce this issue on other Joomla installation?

@sergeytolkachyov
Copy link
Contributor Author

@QuyTon is there a ready-made preset of the Joomla code style for Php Storm? I use PSR-12 for formatting code, but at the same time, the alignment of the code does not occur as (as far as I understand) it is required in the Joomla core. This results in a lot of unnecessary commits.

@sergeytolkachyov
Copy link
Contributor Author

@brianteeman

@rdeutz rdeutz added the Feature label Mar 5, 2025
@rdeutz rdeutz changed the base branch from 5.3-dev to 6.0-dev March 5, 2025 14:56
@rdeutz rdeutz requested review from laoneo and rdeutz as code owners March 5, 2025 14:56
@rdeutz
Copy link
Contributor

rdeutz commented Mar 5, 2025

All new feature have to go into 6.0, rebased the PR, sorry

@sergeytolkachyov
Copy link
Contributor Author

sergeytolkachyov commented Mar 5, 2025

All new feature have to go into 6.0, rebased the PR, sorry

https://joomlacommunity.cloud.mattermost.com/main/pl/ja16arwf7jrrxkqfmxs8dqo6dw

image

Is there any generally accepted approach in this regard? In the semver yes, this PR should get into 6.0. However, I was informed in the chat that there was a chance to get into 5.3 (see screenshot). I decided to abandon a third-party plugin on my projects that implements this functionality and add it to the core. And I tried to make it just because I was informed that it was possible to add it to 5.3, which will be released soon (April). I'm not sorry for the work done, and I'm glad that after successful testing, I hope it will be included in the core. But now I have to wait until the fall and use a third-party extension... And also make sure that maybe some other PR can do about the same thing or make changes that may affect the already tested code.

@brianteeman
Copy link
Contributor

not sure what you are asking me for. I tested this and reported a bug and there have been no changes by you since then so nothing will have changed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet