'multiple' property of MediaUpload misdocumented as boolean

[LuigiPulcini]

The multiple property of the MediaUpload is mistakenly documented as being boolean here in Gutenberg and in several different WordPress files.

In the gallery-add.js controller, the property is even commented as follows: @todo 'add' doesn't seem to do anything special, and gets used as a boolean.

It is worth noting, though, that the property accepts both true and the string value 'add' with two noticeably different behaviors:

– multiple = true: makes it possible to select multiple items in the Media library but, by default, clicking on the thumbnail of an item automatically deselect the previously selected item. In order to select multiple items, it is necessary to click with SHIFT or CMD/CTRL

– multiple = 'add': makes it possible to select multiple items in the Media library and items keep being added simply clicking on their thumbnails, without any need to use the SHIFT or the CMD/CTRL key. In order to deselect a selected item, one can simply click on its correspondent check mark.

I believe this difference could be better documented in both projects.

[youknowriad]

In my testing it's not a documentation issue and the behavior is similar in both cases.

[LuigiPulcini]

@youknowriad, I am not sure what do you mean by 'similar' behavior. Of course, in both cases, you end up selecting multiple files. But the WAY you are selecting them while the Media modal is still open is completely different.

If multiple is set to TRUE (boolean), if you want to select multiple items, you need to click with SHIFT or CMD/CTRL (otherwise, a new selection will automatically deselect any previously selected item).

If multiple is set to add (string) you can freely select multiple items without pressing SHIFT or CMD/CTRL.

This also affects how the Gutenberg MediaPlaceholder component behaves.

[LuigiPulcini]

I have just tested this again in a Gutenberg block for an audio playlist plugin that I am developing and I can confirm the difference between TRUE and add when initializing a new MediaPlaceholder object.

[youknowriad]

I'm reopening for further investigation and clarification.

[LuigiPulcini]

The way I found out while testing my Gutenberg block and a possible way of reproducing both the behaviors is creating a new block based on the source of the Gutenberg Audio block.

Then, in the edit.js file, inside the render method, you alternatively add multiple={ true } or multiple={ 'add' } to the MediaPlaceholder component definition.

If you have more than one audio file in your Media library, the different behavior while clicking on the item thumbnails in the Media dialog will be pretty evident.

[tcmulder]

I'm coming across this also for the MediaPlaceholder docs. In my use case, it's going to be easier for my users to click multiple images they want selected rather than training them to hold the shift key first, and since its not documented I wouldn't have know about multiple = 'add' had I not come across this issue.

[ghost]

The Typescript types for this component don't allow a string on the multiple prop either...

[LuigiPulcini]

It's remarkable how it is taking more than four years to even just acknowledge the different behavior.

My main concern is that, sooner or later, someone could simply ignore this report, close the issue and force multiple to be just boolean... with a great loss of a very useful behavior.

[mikachan]

I've taken a longer look into this issue, and I have a few questions/thoughts:

• The multiple attribute is from the InputHTMLAttributes< HTMLInputElement > element and is 'officially' a boolean, based on the HTML docs.
• Does the behaviour between true and add need to be different, or could we change the behaviour of true to behave like add?

Because the 'official' type of multiple is a boolean, I'm hesitant to encourage the use of a string here. I believe this is adding complexity and perhaps not for much benefit, as the two behaviours are so similar. What do you think of combining the behaviours of true and add, so that both use this logic:

If multiple is set to add (string) you can freely select multiple items without pressing SHIFT or CMD/CTRL.

If we need control over this behaviour, I'd prefer to pass a custom prop to handle this, rather than using multiple.

[LuigiPulcini]

Hi, @mikachan,

Thanks for listening to my observations at WCEU2023.

The multiple attribute is from the InputHTMLAttributes< HTMLInputElement > element and is 'officially' a boolean, based on the HTML docs.

Honestly, I find this a rather weak argument. Although multiple has the same name as an official HTML attribute for input and select elements, it is not clear why it should be seen as derived from there in the context of the CollectionAdd or GalleryAdd controllers, especially when both of them are defined with add as their default value. Those controllers also have several other attributes that are not even listed as possible HTML attributes. Therefore, I don't see a specific reason why multiple should be so strictly connected.

What do you think of combining the behaviours of true and add, so that both use this logic

In addition to the obvious consequences of changing something that has been working like this forever, I can also foresee another problem: the behavior you currently get when multiple is set to add is NOT how a <select> element with multiple works by default. In fact, if you try that, the default behavior is that a new selection replaces any previous selection UNLESS also the CMD/CTRL key is pressed. So, if your previous argument was that multiple is derived from the corresponding HTML attribute of input or select elements and you want to have it as a boolean for that reason, you should also honor the default behavior of <select> elements with multiple. Otherwise, you would only add discrepancies to discrepancies.

I'm hesitant to encourage the use of a string here.

That is a hard battle, when add is the default value for both the GalleryAdd and the CollectionAdd controllers.

I believe this is adding complexity and perhaps not for much benefit, as the two behaviours are so similar.

I truly fail to understand this point about the added complexity: the way this was originally designed beautifully combines two possible behaviors into a single property, giving developers the freedom of picking the one they prefer, as we did for years. For example, I doubt you have never found yourself in a situation where, after selecting dozens of items from a list with hundreds of them, you accidentally click on a new item without having the CMD/CTRL key pressed and have to start from scratch with the whole selection. If you did, you would recognize that those behaviors, despite getting the same ultimate result, are drastically different from a UX point of view.

All that being clarified, if you think that breaking something that has been working like this for as long as the Media dialog exists and adding a new property to achieve the same result is less complex than fixing how this property is documented, I am afraid I do not have much else to add to this conversation.

[mikachan]

Hi @LuigiPulcini, it was good to chat at WCEU2023! Thanks for providing more detail here. I've not looked into this part of the codebase before, but I'm trying my best to progress this issue.

Therefore, I don't see a specific reason why multiple should be so strictly connected.

My reason for suggesting linking this version of multiple with the HTML attribute of the same name was because the type definition for the FormFileUpload component uses this HTML attribute as its type, which is used by the MediaPlaceholder component, mentioned earlier in this issue. I'm not sure how connected these components should be to the CollectionAdd and GalleryAdd controllers, but I believe this type definition is what is causing the TypeScript errors mentioned above. Perhaps these are two separate issues.

you should also honor the default behavior of select elements with multiple

If we want to honor the default behaviour, then we should keep both types of select behaviour, and not merge them into one.

if you think that breaking something that has been working like this for as long as the Media dialog exists and adding a new property to achieve the same result is less complex than fixing how this property is documented, I am afraid I do not have much else to add to this conversation.

To clarify, my intention wasn't to break existing behaviour, but rather to discourage the use of this attribute for multiple purposes going forward, as it's currently being used to determine whether multi-select is enabled, and to inform the behaviour of the multi-select.

It's clear that there is long-standing confusion around what type this attribute is, as seen by the incomplete docs. If we address this by updating documentation, then I believe there are several areas that need updating:

• [x] Update the MediaPlaceholder readme to include 'string' as a type for multiple: https://github.com/WordPress/gutenberg/pull/53350
• [x] Cast 'add' string as a boolean in FormFileUpload: https://github.com/WordPress/gutenberg/pull/53350
• [x] Update the MediaPlaceholder types so that multiple can be a boolean or string: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/66596
• [ ] Update the GalleryAdd controller docs and CollectionAdd controller docs to include 'string' as a type for multiple (these would be changes in Core, not Gutenberg)

Do we need to update any instances of multiple being passed or set within the editor? I've noticed that multiple is often only treated as a boolean, so we may need to tweak some logic to handle strings as well, e.g. in the case where 'add' and true would both be truthy, the behaviour will currently be the same. Maybe this is OK as it's the current behaviour.

I'm hoping this list will inform how best we can address this. It would be great to hear from others if we need to address any other changes.

[LuigiPulcini]

Hi, @mikachan, and thanks again for looking further into this issue.

I'm not sure how connected these components should be to the CollectionAdd and GalleryAdd controllers, but I believe this type definition is what is causing the TypeScript errors mentioned above.

I am inclined to say that the connection between CollectionAdd and GalleryAdd on one side and FormFileUpload on the other side should be considered pretty loose. The reason, in my opinion, is that the former components are not necessarily used to upload files. But I understand the need of uniforming and matching the type definition across components that use similar attributes.

If we want to honor the default behaviour, then we should keep both types of select behaviour, and not merge them into one.

Yes, please! I find – and believe people dealing with galleries or playlists would probably agree with me – that the add behavior is so useful that it would be a shame to break it by flattening the multiple attribute as a boolean.

To clarify, my intention wasn't to break existing behaviour, but rather to discourage the use of this attribute for multiple purposes going forward, as it's currently being used to determine whether multi-select is enabled, and to inform the behaviour of the multi-select.

I totally understood your point. What I was saying is that you could break quite a lot of websites by simply enforcing a boolean value for multiple and creating a new property to differentiate between the two selecting behaviors at hand, as your initial comment seemed to suggest (i.e. I'd prefer to pass a custom prop to handle this, rather than using multiple.).

Do we need to update any instances of multiple being passed or set within the editor? I've noticed that multiple is often only treated as a boolean, so we may need to tweak some logic to handle strings as well, e.g. in the case where 'add' and true would both be truthy, the behaviour will currently be the same.

I don't think so. As you mentioned, the current code of the components we discussed here only uses multiple as boolean and you cannot find a single condition such as 'add' === multiple. I myself have not been able to find any reference to add in the context of the multiple attribute for those two components, other than in the initialization of the controllers. My guess, though, is that somewhere, somehow the add value for multiple is translated into something else that, ultimately, determines the different behavior. For example, whichever intricacy of the implementation leads to this conditional clause of the Attachment View, that is where the magic is happening.

In conclusion, my point is: although we don't see a condition such as 'add' === multiple anywhere in the code and the multiple attribute of those two controllers seems to be used as a plain boolean across the source code, there must be a place where true vs add are translated into the behavior we can clearly observe.

[mikachan]

Hi @LuigiPulcini 👋 Just wanted to highlight some updates on this issue. I believe all of the Gutenberg-related updates have now been merged, including an update to the type definitions and related documentation. These are scheduled to be released with Gutenberg 16.7.

The remaining changes are in the documentation in the Core repo, which I'm planning to work on next. I'm tracking all the work in this comment.