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

Tag form field type removing selected tags from nested multiselect list #43705

Open
rogercreagh opened this issue Jun 25, 2024 · 10 comments
Open

Comments

@rogercreagh
Copy link

rogercreagh commented Jun 25, 2024

Steps to reproduce the issue

In J5.1.1 have a tag form field with mode="nested" and multiple="true" attributes
Use the field to select a tag with children

Do the same in J3.10

Expected result

J5 should still list the selected item, but grey'd out so it can't be double selected but you can still see the nested structure.

This was the behaviour in J3, and having the selected item removed from the list means that its child tags now appear as children of the tag above its original place in the list. Great confusion for user.

Actual result

The selected tags are removed from the dropdown list so the user no longer sees the correct nested structure

System information (as much as possible)

J5,1,1
Apache 2.4.59 on Ubuntu
Php 8.1.29, same on 8.2

Additional comments

see attached screenshots
Joomla 5 before selection of a tag:
J5tag-before-select

Joomla 5 after selection of a tag. Note that the child tags of "Love" now appear as if children of "Worldwide"
j5tag-after-select

For coparisson the J3 version after selecting a tag still shows it in place in the list but grey'd out so that you can still see the child relationships correctly
J3tag-nested-multi

@rogercreagh
Copy link
Author

rogercreagh commented Jun 26, 2024

By the way also note that in J3 when using nested tags the selected tag has hyphens prefixed to indicate the level. This is also useful. Edited : My bad, it does do this ok.

@rogercreagh
Copy link
Author

This must be happening in the new javasript that has replaced the J3 chosen behaviour. Not sure where that is?

@Fedik
Copy link
Member

Fedik commented Jul 14, 2024

Yeah. The script for dropdown comes from Choices.js https://github.com/Choices-js/Choices
And the issue you found, also belong to them.
You can try to report the issue there.

I would close current issue, as it is not Joomla issue.

@rogercreagh
Copy link
Author

rogercreagh commented Jul 15, 2024

Thanks @Fedik . In the short to medium term while they understand what the bug is and accept it (or if they reply saying that is the intended behaviour) is there any way to disable choices.js on a view and reinstate the behaviour.chosen (?bootstrap) script.

I guess simply removing choices.js would break other things, so it need to be corrected on a view or form field level.
Waiting for a short while before closing in case anyone has any suggestions please.
see below - it is a Joomla iisue

@rogercreagh
Copy link
Author

CORRECTION - It is a Joomla issue

Choices.js has an option:

renderSelectedChoices
Type: String Default: auto

Input types affected: select-multiple

Usage: Whether selected choices should be removed from the list. By default choices are removed when they are selected in multiple select box. To always render choices pass always.

So joomla needs to set renderSelectedChoices to always when setting up choices.js for nested tag fields. It will also need to reinstate the css from J3 in the default templates to grey out selected (checked) items in the list.

Or something like that. (obviously if the mode for the field ia ajax then you probably want renderSelectedChoices set to auto

@rogercreagh
Copy link
Author

Ok so I can see how to fix it for nested tags, but not being that familiar with Joomla core dev decisions, or testing procedures, or javascript I am not competent to work this up into a PR.

Here's what I have found.
First to restate the problem. In a nested mode multiple selection tag form field Choices.js is removing the selected items from the select list.
This is correct behaviour in ajax mode, but in nested mode it destroys the hierarchy in the list. In this mode selected items should be greyed out not removed. Choices.js is capable of doing this by setting renderSelectedChoces="always"

The joomla interface to Choices.js for this is handled by /media/system/js/fields/joomla-field-fancy-select.js (and the ... .min.js and ... .min.js.gx as appropriate of course)

In the case of tags it seems to me that if mode="nested" then you will never want selected options removed from the list. So the issue is to pass the mode setting to joomla-field-fancy-select.js and use it to setrenderSelectedChoices as appropriate.

NB what follows is specific to the tag form field type - I am not sure what other fields types have a nested mode and are using fancy-select so would need the same treatment.

Solution;

  1. In layouts/joomla/form/field/tag.php you need to detect the $isNested state from the field and pass it on to the javascript.
    at about line 88 insert the following:
if ($isNested) {
  $attr2 .= ' render-selected="always"';
}
  1. Also in the same file add to the commented list at about line 54 insert
    * @var string $isNested true if the mode is "nested", false if it is "ajax"

  2. In /media/system/js/fields/joomla-field-fancy-select.js at about line 62 insert:

	get renderSelectedChoices() {
		return this.getAttribute('render-selected') || 'auto';
	}

Probably also add a comment at about line 20 documenting what this is for

  1. Those two additions will ensure that in a nested tag field selected items are not removed from the list. Now we need to add some css to make it clear that these items are not available for selection. I am unsure where this would best sit - in the templates, in a specific field css, in choices.css (I don't see anything appropriate there, but might have missed it).
    The selected items have a class is-selected so it is easy enough to add css to dim it.

For a quick hack inserting the following at about line 124 (accounting for the addition above) in layouts/joomla/form/field/tag.php works fine:

<style type="text/css" media="screen">
	.is-selected { 
            color: #686;
            cursor: none;
        }
</style>

As I said above this works fine for nested tag fields, It needs someone with an understanding of what other things might be impacted to work this into a PR - I'm afraid that is somewhat beyond my paygrade, but I hope this helps for starters.

@Fedik
Copy link
Member

Fedik commented Jul 17, 2024

Thanks for finding.

Just add it here

this.choicesInstance = new Choices(this.select, {
placeholderValue: this.placeholder,
searchPlaceholderValue: this.searchPlaceholder,
removeItemButton: true,
searchFloor: this.minTermLength,
searchResultLimit: parseInt(this.select.dataset.maxResults, 10) || 10,
renderChoiceLimit: parseInt(this.select.dataset.maxRender, 10) || -1,
shouldSort: false,
fuseOptions: {
threshold: 0.3, // Strict search
},
noResultsText: Joomla.Text._('JGLOBAL_SELECT_NO_RESULTS_MATCH', 'No results found'),
noChoicesText: Joomla.Text._('JGLOBAL_SELECT_NO_RESULTS_MATCH', 'No results found'),
itemSelectText: Joomla.Text._('JGLOBAL_SELECT_PRESS_TO_SELECT', 'Press to select'),
// Redefine some classes
classNames: {
button: 'choices__button_joomla', // It is need because an original styling use unavailable Icon.svg file
},
});

renderSelectedChoices: 'always',

It will be fine as default. As long as it is not selectable after initial selection.
No need all that complication.

@rogercreagh
Copy link
Author

Hmm, my feeling is that if the mode is not nested (ie mode="ajax" in the field definition) then selected items should be removed from the list. In a tag field certainly you are not going to want to select the same tag twice (and anyway the second selection would be ignored).

It is much better for the user in that case if the already selected items are removed from the list. (this was the case in J3)

So you do need to only do it when the mode is nested, hence the change to the tag layout file to get the value to pass to fancy-select, and to get the value to conditionally set renderSelectedChoices in fancy-select

In the nested case where selected items remain in the list then they must be greyed out as not selectable - so you still need to add the .is-selected {} css somewhere. If doing it tag layout file then you probably want a light grey colour so as not to class with other eleement although I prefer a light green to indicate it is already selected. Again this was default in J3.

@rogercreagh
Copy link
Author

There are also two further bugs in the tag field definition itself to do with displaying the list which I will raise as a separate issue as they are independent of this one.

@Fedik
Copy link
Member

Fedik commented Jul 17, 2024

You can use if(this.remoteSearch) to check if AJAX search enabled

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants