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 31 commits into
base: 5.3-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

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-5.3-dev labels Feb 25, 2025
@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.

@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.

@sergeytolkachyov
Copy link
Contributor Author

I have tested this item ✅ successfully on 8a2e00b


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 8a2e00b


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

@web-eau-net
Copy link

I have tested this item ✅ successfully on 8a2e00b

Nice job!


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

@brianteeman
Copy link
Contributor

I am sorry but unless I misunderstand something this creates a very confusing UI in the admin and broken output in the front end and I dont see how any of you can have tested this successfully

image

image

image

image

@brianteeman
Copy link
Contributor

I have tested this item 🔴 unsuccessfully on 8a2e00b


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

@brianteeman
Copy link
Contributor

gets a bit confused if you select images + another type

image

@brianteeman
Copy link
Contributor

brianteeman commented Feb 27, 2025

php warnings

Create a field for documents and in an article select a pdf and save
The following php warning
[27-Feb-2025 14:24:45 UTC] PHP Warning: mime_content_type(images/RL%20application%202017.pdf): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99

Same for video
Feb-2025 14:24:08 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

image

@brianteeman
Copy link
Contributor

From an accessibility perspective the default text for every link is "download"
This can result in multiple links on the same page with the exact same details - which is an accessibility failure

An option might be to do it the way we do a read more link. eg Download < Filename >

Tagging @chmst for their a11y input

@brianteeman
Copy link
Contributor

@viocassel how are you testing this?

@sergeytolkachyov
Copy link
Contributor Author

php warnings

Create a field for documents and in an article select a pdf and save The following php warning [27-Feb-2025 14:24:45 UTC] PHP Warning: mime_content_type(images/RL%20application%202017.pdf): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99

I can't repeat the problem. Perhaps this is a MediaHelper and location issue on Windows.

image

Same for video Feb-2025 14:24:08 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

image

@sergeytolkachyov
Copy link
Contributor Author

gets a bit confused if you select images + another type

image

Yes, I agree. Showon comes to mind here, but there is no substring search so that you can show the rest of the fields based on the value of the main one.

@brianteeman
Copy link
Contributor

for video and audio I would expect the output to be a player not a download.

see

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/video
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/audio

@sergeytolkachyov
Copy link
Contributor Author

for video and audio I would expect the output to be a player not a download.

see

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/video https://developer.mozilla.org/en-US/docs/Web/HTML/Element/audio

To do this, I will need to add new layouts. However, even for pdf, you will want to show them as an <object> instead of a download link. Also, each file type may require its own additional data and settings specific to that particular file type. For videos, for example, the poster attribute or specifying a file in a different format (mp4, webm etc.). This can greatly complicate the logic of choosing the output layout. I assumed that at this point, the very fact that you can choose something other than pictures is already a good thing. And the rest can be solved using override.

@sergeytolkachyov
Copy link
Contributor Author

image
What doesn't it like?

@brianteeman
Copy link
Contributor

If a user selects an image and it displays an image then when they select a video they would expect it to display a video etc overrides are for customising the output with a layout is fine but it should work out of the box without an override.

@sergeytolkachyov
Copy link
Contributor Author

sergeytolkachyov commented Feb 27, 2025

If a user selects an image and it displays an image then when they select a video they would expect it to display a video etc overrides are for customising the output with a layout is fine but it should work out of the box without an override.

So we need a isVideo, isDocument and isAudio methods for MediaHelper... And check mime types too.
Because in plugins/fields/media/tmpl/media.php I got a field params (with all file types configured for it) and field value. But I don't get the exact file type in the layout. Therefore, I need to define it, and then generate the necessary options for the render.

Alternatively, I can realize this check in the plugins/fields/media/tmpl/media.php for now, and then add the isAudio, isDocument and isVideo methods to MediaHelper. After adding these methods, it will be possible to simplify the verification in the plugin layout.

@sergeytolkachyov
Copy link
Contributor Author

If a user selects an image and it displays an image then when they select a video they would expect it to display a video etc overrides are for customising the output with a layout is fine but it should work out of the box without an override.

I added this. Check it please.

@sergeytolkachyov
Copy link
Contributor Author

Testing instructions updated

@sergeytolkachyov
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators PR-5.3-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants