[4.1] Form list with showon attributes should load the show on script

[dgrammatiko]

Pull Request for Issue #37299

Summary of Changes

• Load the showon script if there are showon attributes even in the options (one level down)

Testing Instructions

• Open file: \joomla-cms\administrator\components\com_banners\forms\banner.xml
• Find the state field and replace with the following

		<field
			name="state"
			type="list"
			label="JSTATUS"
			class="form-select-color-state"
			default="1"
			validate="options"
			showon="state!:X"
			>
			<option value="1" showon="sticky:0">JPUBLISHED</option>
			<option value="0" showon="sticky:0">JUNPUBLISHED</option>
			<option value="2" showon="sticky:0">JARCHIVED</option>
			<option value="-2" showon="sticky:0">JTRASHED</option>
		</field>

• Create a new banner: /administrator/index.php?option=com_banners&view=banner&layout=edit
• Toggle Banner's Pinned button. You'll see the Status options disappear after toggling. (You'll first have to click the status list in order to see it change. It would be better for it to update, but that's another issue)
• Now remove the showon="state!:X" line from the code above. Press CTRL+F5.
• Retry toggling, it's broken now.

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Documentation Changes Required

[toroworx]

I just tested this patch and can confirm it fixes the issue.
Thanks for creating a patch! Awesome feature btw.

[richard67]

@toroworx Could you go to the PR in the issue tracker here https://issues.joomla.org/tracker/joomla-cms/37300 and use the "Test this" button to submit your test result? Thanks in advance.

[toroworx]

I have tested this item ✅ successfully on ac8bed2

I just tested this patch and can confirm it fixes the issue. After the patch you don't need to manually load the showon.min.js script anymore.

I just found out though: to replicate the issue, you have to make sure there's not a field with a showon attribute, else this will load the showon.min.js script, preventing you from replicating the issue. Nevertheless the patch works fine.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37300.}

[dgrammatiko]

I have changed this PR so it will use the Platform wide $options['showonEnabled']

joomla-cms/libraries/src/Form/FormField.php

Lines 1070 to 1075 in 29ec453

	if ($this->showon)
	{
	$options['rel'] = ' data-showon=\'' .
	json_encode(FormHelper::parseShowOnConditions($this->showon, $this->formControl, $this->group)) . '\'';
	$options['showonEnabled'] = true;
	}

which is held automatically here

joomla-cms/layouts/joomla/form/renderfield.php

Lines 28 to 33 in 29ec453

	if (!empty($options['showonEnabled']))
	{
	/** @var Joomla\CMS\WebAsset\WebAssetManager $wa */
	$wa = Factory::getApplication()->getDocument()->getWebAssetManager();
	$wa->useScript('showon');
	}

[richard67]

@toroworx Could you test again with the latest changes? Thanks in advance.

[toroworx]

Seems to be broken right now.

Test instructions:

1. Open file: \joomla-cms\administrator\components\com_banners\forms\banner.xml
2. Find the state field and replace with the following

		<field
			name="state"
			type="list"
			label="JSTATUS"
			class="form-select-color-state"
			default="1"
			validate="options"
			showon="state!:X"
			>
			<option value="1" showon="sticky:0">JPUBLISHED</option>
			<option value="0" showon="sticky:0">JUNPUBLISHED</option>
			<option value="2" showon="sticky:0">JARCHIVED</option>
			<option value="-2" showon="sticky:0">JTRASHED</option>
		</field>

3. Create a new banner: /administrator/index.php?option=com_banners&view=banner&layout=edit
4. Toggle Banner's Pinned button. You'll see the Status options disappear after toggling.
   (You'll first have to click the status list in order to see it change. It would be better for it to update, but that's another issue)
5. Now remove the showon="state!:X" line from the code above. Press CTRL+F5.
6. Retry toggling, it's broken now.

[richard67]

@toroworx Have you applied the changes in ListField.php AND also reverted the List.php to the original state like without this PR? Or have you maybe forgotten the latter?

[laoneo]

@toroworx what do you expect instead of that the status are hidden? For me it works exact the same way as you did describe it in the last comment which is for me the expected behavior.

[toroworx]

@richard67 I've used the Joomla Patch Tester. I reverted the patch, fetched data, and applied the patch.
Just to be sure I undid all that. Switched to the patch-17 branch, pulled in the latest version and retested (so testing 723f805). Again I can confirm the latest patch-17 version is broken.

@laoneo did you refresh after step 5? The definition of broken is: not hiding the Status options any more after toggling the banner's Pinned button (you have to click the Status option to view if the options are hidden). To clarify, this only happens if only option showon is used. So like this:

<field
		name="state"
		type="list"
		label="JSTATUS"
		class="form-select-color-state"
		default="1"
		validate="options"
>
	<option value="1" showon="sticky:0">JPUBLISHED</option>
	<option value="0" showon="sticky:0">JUNPUBLISHED</option>
	<option value="2" showon="sticky:0">JARCHIVED</option>
	<option value="-2" showon="sticky:0">JTRASHED</option>
</field>

If on the page somewhere showon is used within a field tag, the showon is loaded and the issue can't be replicated.

[dgrammatiko]

@toroworx you're right the latest patch is not working as expected

@laoneo there's problem with the List field (which is also the base for many other fields). The showon is loaded only if the field is declaring a showon attribute in the root, if it is in a nested option the showon is not loaded. This affects many other fields. My first approach was naive, eg have a new option passed in the layout data but I realised that will not work for any of the fields that are extending the List field. Also I have no clue how this can be achieved without some refactoring (eg the getLayoutData or the getOptions fn)

[laoneo]

Thanks for clarification, now I got it.

[laoneo]

@dgrammatiko did your last changes fix the bug which was found by @toroworx?

[dgrammatiko]

@laoneo it should solve the reported issue, if not then it should be closed

[toroworx]

/edited:
I just pulled the latest commits of your patch-17 branch and tested it. It doesn't fix it. Still same state as before the patch. showon.min.js is not loaded. It's still acting like explained above.

I appreciate your pull request, but why do you suggest it's fixed before testing it? With the provided instructions it shouldn't take long.

[dgrammatiko]

but why do you suggest it's fixed before testing it? With the provided #37300 (comment) it shouldn't take long.

I did and the showon file was loaded. Anyways someone else could have better lack with this

[toroworx]

I did and the showon file was loaded. Anyways someone else could have better lack with this

Okay, I'm sorry then. Weird it did work for you and not for me.

@dgrammatiko I've edited your approach and this works for me now. Can you please review the approach?

toroworx@d428b1f

[richard67]

I‘ve re-opened the issue. But @dgrammatiko could you check the commit linked by @toroworx ?

[richard67]

Meanwhile @toroworx has made a PR, see #37451 .

[toroworx]

Meanwhile @toroworx has made a PR, see #37451 .

Thanks for reopening @richard67. I don't want to hijack anything, if I'll need to close something please let me know!