User tests: Successful: Unsuccessful:
Pull Request for Issue #37299
\joomla-cms\administrator\components\com_banners\forms\banner.xml
<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>
/administrator/index.php?option=com_banners&view=banner&layout=edit
showon="state!:X"
line from the code above. Press CTRL+F5.Status | New | ⇒ | Pending |
Category | ⇒ | Layout |
Labels |
Added:
?
|
Category | Layout | ⇒ | Layout Libraries |
@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.
I have tested this item
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.
Category | Layout Libraries | ⇒ | Libraries |
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
joomla-cms/layouts/joomla/form/renderfield.php
Lines 28 to 33 in 29ec453
Seems to be broken right now.
<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>
Pinned
button. You'll see the Status
options disappear after toggling.showon="state!:X"
line from the code above. Press CTRL+F5.@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.
@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)
Thanks for clarification, now I got it.
@dgrammatiko did your last changes fix the bug which was found by @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.
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
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-04-01 15:33:49 |
Closed_By | ⇒ | dgrammatiko |
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?
I‘ve re-opened the issue. But @dgrammatiko could you check the commit linked by @toroworx ?
Thanks for reopening @richard67. I don't want to hijack anything, if I'll need to close something please let me know!
I just tested this patch and can confirm it fixes the issue.
Thanks for creating a patch! Awesome feature btw.