multiselect.js issue with unwanted selections

[rogercreagh]

Steps to reproduce the issue

In a component admin list view with select checkbox in the first column of each row and multiselect.js loaded.

include a <details><summary>...</summary>...</details> element in a column

include an <a ...><span>...</span>...</a> element in a column

Expected result

clicking in the <details> element to open or close the details should not toggle the checkbox in the first column

clicking the <span> inside the <a> should not toggle the checkbox

Actual result

In both cases the checkbox is toggles. In the case of the span (or any other element inside the text for an <a>) it is only a potential problem if the target is for a new tab/window.

System information (as much as possible)

Joomla 5.1

Additional comments

In principle you might think that adding 'DETAILS' and 'SUMMARY' to the if (... statement on line 52 in multiselect.js might fix the problem. However in real use cases you should be able to click anywhere in details area once it is open without toggling the checkbox..

In practice there are very often child elements (eg a list) inside the details and the if statement on line 52 as written is only checking the actual element that generates the event. It needs to be checking also the parents of the target in order to determine if the click is taking place inside one of the excluded elements.

This is why the span, or any other element, inside the text for an <a> causes the checkbox to be toggled.

A common case for using a <span> inside an <a> is if you want to use a font-awesome icon for the link.

A workaround is to add a stopPropogation(event); action in a function for the parent element onclick() event, but this shouldn't be necessary on a case by case basis. The multiselect.js function should exclude <details> elements and child elements inside excluded elements should also be excluded.

[brianteeman]

Unless I am mistaken this can only occur in a non-core component.

Of course the alternative to rewriting the core js is for you to use your own js to satisfy the requirements of your own component.

[rogercreagh]

@brianteeman you are quite correct, but it is a bit disappointing that Joomla now provides seemingly generic library facilities like multiselect.js which only work with core components, or with components that don't do anything that the core components don't do.

Including <details> facility of html, particularly in table lists where you might want the user to be able to see more detail in a particular row whilst maintaining a reasonably compact set of rows, is a very good design feature which improves the user experience. I can think of cases where this would be a useful in the core components as well.

There are similar issues with the table.columns script. It breaks if you include a <tfoot>, or a <colgroup> in your table, both of which are very useful standard html facilities. I use table footers to repeat the content of the header if the table is long enough to cause the page to scroll. Thus when you scroll to the bottom of the table you can still see the column titles and access the sorting facilities. The core Articles list table for example would be much improved by this. <colgroup> is great for improving the self-documentation of tables - it avoids having to have classes in the table elements and you can see the whole structure in one place. Again it could usefully be used in core components.

Sticky headers could also solve this problem, but I've yet to find a smooth implementation - and a sticky header would also make the non-core component out of line with the UI of the core components; it would have to be done site wide. But including table footers and details elements in cells remains broadly in line with the core design - extending, rather than changing it.

table.columns also breaks if you use <colspan>s, but in Joomla list tables this is less likely to be something you need in an admin list table.

In summary it seems to me that it is reasonable for a component designer to expect the core features of joomla libraries to work as documented. As a minimum the "bug" could be fixed by making it very clear in the documentation what the limitations are.

I really like these new facilities in J4/5 and it is a great shame that they can't simply be used in custom extensions, or indeed in layout overrides to core components.

[Fedik]

I set it as a bug, as it definitely can be improved. If you know how to fix it, you can make a PR. Thanks.

Hint: There already a code for similar cases: https://github.com/joomla/joomla-cms/blob/beb3b34c7cf90548af30648b25d28261c1b540b3/build/media_source/system/js/multiselect.es6.js#L48-L54 Which need to be update.

[rogercreagh]

I did try to fix it, but it gets quite complex having to deal with parent elements. Simply adding || target.tagName === 'DETAILS' || target.tagName === 'SUMMARY' doesn't work in cases where either the summary or details contain anything other than plain text. Likewise the case where <a> text contains html - and probably the same for <button>

In the meantime I'm using stopPropogation() as a short term dirty fix and will get back to looking at this once I've finished current component developments (could be a year or more).

[rogercreagh]

I'll add a separate issue re table.columns.js but I have raised a PR on docmentation to note that one. (there is no manual page for multiselect yet as far as I can find aside from the API docs and various extension development tutorials which use it.

[Fedik]

Please test the fix https://github.com/joomla/joomla-cms/pull/43574

[rogercreagh]

Please test the fix #43574

Brilliant. And super fast fix. I'd not found the element.closest() function in my searching and never heard of it before.

Yep it works. Many thanks.

[Fedik]

@rogercreagh please visit https://issues.joomla.org/tracker/joomla-cms/43574 and push the button. Thanks.

[rogercreagh]

Done thanks, sorry for delay - offline for sunny weekend.