RFC NPM Resource Changed PR-5.2-dev Pending

User tests: Successful: Unsuccessful:

avatar dgrammatiko
dgrammatiko
29 Jul 2024

Pull Request for Issue # .

Summary of Changes

  • Introduce a new url query parameter named selected_items that can take multiple item paths separated by comma (right now it accepts only one)
  • Reflect the selected items (now it takes only one) on the initial loading

If you get it this is a missing part for some coherent UI behavior on the item selection (or items)

Testing Instructions

  • Apply the patch.
  • Login to the admin area
  • Check that these urls have preselected some image
    administrator/index.php?option=com_media&path=local-images:/&selected_items=local-images:/joomla_black.png
    administrator/index.php?option=com_media&path=local-images:/&selected_items=local-images:/banners/osmbanner1.png
    administrator/index.php?option=com_media&path=local-images:/&selected_items=local-images:/sampledata/parks/landscape/180px_ormiston_pound.jpg

Actual result BEFORE applying this Pull Request

Only one way selection (from media manager to the picker)

Expected result AFTER applying this Pull Request

Two way selection (from media manager to the picker and through the url)

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

@laoneo @Fedik couple of things here:

  • Keep the old customEvent till the end of 6.0 but introduce a new one that:

    • The event that media manager is dispatching should be const ev = new Event(); ev.selectedItems = []
    • The name of the event should be more in the format that other core events have (I'm guessing something like ComMedia:selectedItems or something like that @Fedik your input here?)
    • The selectedItems should ALWAYS be an array
    • if !array.lenght nothing selected
    • if array.lenght === 1 ONE item selected
    • if array.lenght > 1 multiple items selected

These changes will allow better integration with 3rd part extensions

avatar dgrammatiko dgrammatiko - open - 29 Jul 2024
avatar dgrammatiko dgrammatiko - change - 29 Jul 2024
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 29 Jul 2024
Category JavaScript Administration com_media NPM Change
avatar dgrammatiko dgrammatiko - change - 29 Jul 2024
The description was changed
avatar dgrammatiko dgrammatiko - edited - 29 Jul 2024
avatar Fedik
Fedik - comment - 30 Jul 2024

I not realy see use case for multiple select in core (at current point of time).
But probably nice to have.

Few notes for it:

  • Maybe make parameter shorter, more generic: files (or file) or items or selected or anything similar.
  • The parameter can be multiple:
const url = new URL('index.php?file=a&file=b', location.origin);
const files = url.searchParams.getAll('file');

// Safe for PHP $_GET
const url = new URL('index.php?file[]=a&file[]=b', location.origin);
const files = url.searchParams.getAll('file[]');

The comma also fine, just have to do extra spliting.


Abut the event. I think it should be window.parent.postMessage().
We already have joomla:content-select messageType (I wanted to change it while working on media select field, but there much much much more stuff need to change) :

window.parent.postMessage({
  messageType: 'joomla:content-select', 
  contentType: 'com_contact.article',
  id: 1, 
  title: 'A title'
})

For multiple I thought about something like:

window.parent.postMessage({
  messageType: 'joomla:content-select', 
  contentType: 'com_contact.article',
  items:[
    {    
     id: 1, 
     title: 'A title'
    }
  ]
})

Or maybe more safe, to have another message type 'joomla:content-select-multiple', not sure curently.
Had no chance to test.

I would skip this part for now, it is not used in core anywhere, it will be hard to test.

avatar dgrammatiko
dgrammatiko - comment - 31 Jul 2024

I not realy see use case for multiple select in core (at current point of time).

@Fedik here's my take on the multiple select:

  • any media picker right now is limited to just selecting 1 item. Assuming you want to build a module for an image slider then the user is forced, due to the limitation of the current API, to pick one image, then add another field (assuming using the repeatable field, but other implementations would fall into the same workflow) then click on the pick button, open the modal select the next image and repeat the flow for as many images are required for the slider. If the API allowed multiple items then it would be click on the picker -> open the modal -> select the images.
  • Allowing multiple items is like orders of magnitude better UX than the current allowed API.

Will it be used by core?

Probably not immediately as it would require a new core field (and|or a custom field)

Anyways the API should not dictate/limit the use cases IMHO

About The parameter can be multiple, you're right, this was a quick mashup to showcase the idea, but agree to use any of the suggested implementations

Abut the event. I think it should be window.parent.postMessage()

I totally forgot that we had this conversation a few years back and decided to go with postMessage. Noted so if this gets a consensus will do it like that.

@laoneo or anyone from the production team has any objections here?

avatar brianteeman
brianteeman - comment - 31 Jul 2024

When selecting multiple images how do you foresee the ability to add alt text descriptions for each image?

avatar dgrammatiko
dgrammatiko - comment - 31 Jul 2024

When selecting multiple images how do you foresee the ability to add alt text descriptions for each image?

The same pattern that exists today for the single image could be done (ok with some obvious changes) to accommodate the case of multiple images. The whole alt/lazyloading/etc right now is a component that accepts a single image, could be adjusted for multiple images, nothing really awkward or especially hard to implement.

FWIW I'm not suggesting changing the current media field or the editor image/media picker, just allow the API to expose multiple selected items. For the field probably a new one should be introduced (questionable if this needs to be in the core), same goes for the editor button(?).

avatar Fedik
Fedik - comment - 31 Jul 2024

any media picker right now is limited to just selecting 1 item. Assuming you want to build a module for an image slider then the user is forced, due to the limitation of the current API, to pick one image

Not fully correct. You can select multiple already.
It is our media field limited to only one (due to hacky approach), but if you want you can.

Here a quick pseudo code, how to:

const myFiles = [];
document.addEventListener('onMediaFileSelected',  (e) => {
  const myFile = e.detail;
  if (myFile in myFiles) {
    //.. remove myFile from the list of selected myFiles
  } else {
   myFiles.push(myFile);
  }
});

// ... then can insert these files anywhere you need
avatar dgrammatiko
dgrammatiko - comment - 31 Jul 2024

but if you want you can.

Sorry but that's too hacky for my taste. Workarounds are always possible but it's better to have a proper API...

avatar Fedik
Fedik - comment - 31 Jul 2024

I just saying that it is already possible.
It is diferent approach, not realy hacky.

With your changes instead of 1 file, you will send all selected on every "select" event.
That is also fine.

avatar dgrammatiko
dgrammatiko - comment - 31 Jul 2024

@Fedik my point is that the media manager already has a store that keeps a reference of the selected items and it's easier to send a post message or an event when that value changes. Right now the media manager has multiple event dispatchers inside components and inside the store. So, one improvement would be to have the event dispatching from one place and have the value standardized to whatever is there (an array of items). This simplifies the MM and the picker would have minimal changes but the selected items would be the same on both sides (the picker could filter items, etc it's just an array)

avatar Fedik
Fedik - comment - 31 Jul 2024

Yeah, I understood.

avatar dgrammatiko dgrammatiko - change - 20 Aug 2024
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2024-08-20 14:12:13
Closed_By dgrammatiko
Labels Added: RFC NPM Resource Changed PR-5.2-dev
avatar dgrammatiko dgrammatiko - close - 20 Aug 2024
avatar dgrammatiko
dgrammatiko - comment - 20 Aug 2024

Too late for 5.2, maybe someone will be interested to bring this in 5.3 or 6.0

Add a Comment

Login with GitHub to post a comment