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

[5.2] Media field: selectable folders #43579

Merged
merged 17 commits into from
Jul 23, 2024
Merged

Conversation

Fedik
Copy link
Member

@Fedik Fedik commented May 30, 2024

Summary of Changes

The PR add posibility to select directories usign the Media field.
This is helpfull when the site have multiple Media adapters.

Testing Instructions

Run npm install.

Create custom field Media,
Pick the directory in the "Directory" option.

Edit TinyMCE (plugin config) "Images Directory" (when "Images Drag and Drop" is On). And test TinyMCE drag and drop upload.

Actual result BEFORE applying this Pull Request

You able to pick only one of folder under /images

Expected result AFTER applying this Pull Request

You can pick any directory that available in the Media manager

Link to documentations

Please select:

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.2-dev labels May 30, 2024
@Fedik Fedik added the Feature label May 30, 2024
@brianteeman
Copy link
Contributor

brianteeman commented May 30, 2024

Works fine for custom fields but what about in the media plugin? These are all subdirectories of /images

image

@dgrammatiko
Copy link
Contributor

@Fedik you could probably patch also this event:


So selecting a folder on the left (disk/folder tree) could be also a valid way to select the folder

@Fedik
Copy link
Member Author

Fedik commented May 30, 2024

@brianteeman I forgot it exists, should be good now.
@dgrammatiko updated

@brianteeman
Copy link
Contributor

Is it possible to use something other than "types" as I am sure people will confuse it with "type"

@dgrammatiko
Copy link
Contributor

Is it possible to use something other than "types" as I am sure people will confuse it with "type"

That would be a b/c break so no

@Fedik
Copy link
Member Author

Fedik commented May 30, 2024

Nope, unfortunately. It is what we already use

$this->types = isset($this->element['types']) ? (string) $this->element['types'] : 'images';

For the Media field.

@brianteeman
Copy link
Contributor

brianteeman commented May 30, 2024

ok fair enough. I searched for types = and got no hits - didn't think about types =

;)

@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label Jun 2, 2024
@Fedik
Copy link
Member Author

Fedik commented Jun 2, 2024

I also updated TinyMCE upload directory for "drag and drop", to use this feature.

Co-authored-by: Quy <quy@nomonkeybiz.com>
@laoneo
Copy link
Member

laoneo commented Jun 5, 2024

Nice work. I tested it with DPMedia and the FTP plugin, unfortunately when I select a folder from the FTP adapter, the field stays empty in the media custom field.

@dgrammatiko
Copy link
Contributor

@laoneo is right, this approach will never work for anything else other than the local filesystem. Maybe having dolder/path#adapterName://folder could work, but would need patches all over I guess

@Fedik
Copy link
Member Author

Fedik commented Jun 5, 2024

Thanks for checking. I did not have a time to test with external adapters, and expected it works.
I will update later

@laoneo
Copy link
Member

laoneo commented Jun 6, 2024

There is a free version of DPMedia with the FTP adapter if you want to try it by yourself.

@Fedik Fedik marked this pull request as ready for review June 7, 2024 14:25
@Fedik Fedik marked this pull request as draft June 7, 2024 14:46
@Fedik Fedik marked this pull request as ready for review June 7, 2024 15:01
@Fedik
Copy link
Member Author

Fedik commented Jun 7, 2024

I have update PR, now the folder should be also selectable on "double click".

@laoneo
Copy link
Member

laoneo commented Jun 20, 2024

This worked perfectly, also with navigation into another folder and with other adapters than the local one.

@laoneo
Copy link
Member

laoneo commented Jul 22, 2024

I have tested this item ✅ successfully on 3223f82

See my last comment


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

@dgrammatiko
Copy link
Contributor

I have tested this item ✅ successfully on 3223f82


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

1 similar comment
@dgrammatiko
Copy link
Contributor

I have tested this item ✅ successfully on 3223f82


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

@Quy
Copy link
Contributor

Quy commented Jul 22, 2024

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 22, 2024
@dgrammatiko
Copy link
Contributor

dgrammatiko commented Jul 22, 2024

Just a note for the person that would merge this PR: depending on the order of committing this PR and #43823 the one that would be second would have to update the extra event introduced in please merge first this PR and then #43823.

@Hackwar
Copy link
Member

Hackwar commented Jul 23, 2024

This feature is nice and I will merge it as soon as drone has passed. One thing which I find strange however is, that you can set it to a subfolder, but then you can still move up the folders and select a different folder. This is also present in the current implementation, but if I set it to folder X, I wouldn't expect to be able to upload in a parent folder of that.

@Hackwar Hackwar merged commit 3cbf8f6 into joomla:5.2-dev Jul 23, 2024
3 checks passed
@Hackwar Hackwar added this to the Joomla! 5.2.0 milestone Jul 23, 2024
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 23, 2024
@Hackwar
Copy link
Member

Hackwar commented Jul 23, 2024

Thanks @Fedik for your contribution. :-)

@Fedik Fedik deleted the media-folder-select branch July 24, 2024 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Required Feature Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.2-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants