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

[#33295] Adding the "showon" & hiddenLabel feature to JForm #3379

Closed
wants to merge 7 commits into from

Conversation

Projects
None yet
@chivitli
Copy link
Contributor

chivitli commented Mar 27, 2014

This is like PR #3127, just in sync with the latest changes, and it also adds support for hidden labels on xml level. Here is the copy of the test instructions/description from the previous PR:

Overview

In the Joomla Global Configuration we use showon attribute to show/hide certain fields based on the state of a "parent" field. You can see this for example with the FTP settings where the settings itself only show when "Enable FTP" is set to "Yes". It's also available for Component settings.

This PR takes this feature to the JForm so it can be used in any xml file.

Testing

You can add a showon attribute to any config field you want to hide. For example in
modules\mod_finder\mod_finder.xml you add to line 94
showon="show_label:1". So it looks like this:

                <field
                    name="label_pos"
                    type="list"
                    default="left"
                    showon="show_label:1"
                    label="MOD_FINDER_FIELDSET_ADVANCED_LABEL_POS_LABEL"
                    description="MOD_FINDER_FIELDSET_ADVANCED_LABEL_POS_DESCRIPTION">
                    <option
                        value="right">JGLOBAL_RIGHT</option>
                    <option
                        value="left">JGLOBAL_LEFT</option>
                    <option
                        value="top">MOD_FINDER_CONFIG_OPTION_TOP</option>
                    <option
                        value="bottom">MOD_FINDER_CONFIG_OPTION_BOTTOM</option>
                </field>

This will only show the "Label position" setting only if the "Show Label" is set to "Yes" (which has the value 1).

Tracker: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=33295&start=0

chivitli added some commits Mar 27, 2014

Adding the "showon" feature to JForm
Please check PR #3127 for details and test instructions, this is the same thing just in sync with the latest changes. It also adds support for hidden labels.

@chivitli chivitli changed the title Adding the "showon" & hiddenLabel feature to JForm [#33295] Adding the "showon" & hiddenLabel feature to JForm Mar 27, 2014

@wilsonge

This comment has been minimized.

Copy link
Contributor

wilsonge commented Mar 27, 2014

Looks good :) Will test this as soon as I get home tonight!

$options['hiddenLabel'] = true;
}
if ($showon = $this->getAttribute('showon')) {

This comment has been minimized.

Copy link
@wilsonge

wilsonge Mar 27, 2014

Contributor

This if statement should go inside the JLayout so people can override the JS if they really want to

This comment has been minimized.

Copy link
@chivitli

chivitli Mar 27, 2014

Author Contributor

I don't think that the if statement can go inside the layout, as inside of it there are calls to member functions, however I can move JHtml calls to layout file, if that's satisfying?

This comment has been minimized.

Copy link
@wilsonge

wilsonge Mar 27, 2014

Contributor

Ok yeah that's fine!

This comment has been minimized.

Copy link
@chivitli

chivitli Mar 27, 2014

Author Contributor

Done, please have a look again

chivitli added some commits Mar 27, 2014

<div class="control-group">
<?php if (!isset($displayData['hiddenLabel']) || isset($displayData['hiddenLabel']) && $displayData['hiddenLabel'] == false) : ?>
<?php
if(!empty($displayData['options']['showonEnabled']))

This comment has been minimized.

Copy link
@Bakual

Bakual Mar 27, 2014

Contributor

Can you please put a space after the if, like if (!empty...

This comment has been minimized.

Copy link
@chivitli

chivitli Mar 27, 2014

Author Contributor

Yes, I added it. It takes time to get used to a different coding style...

This comment has been minimized.

Copy link
@Bakual

Bakual Mar 27, 2014

Contributor

👍

@@ -900,8 +900,25 @@ public function renderField($options = array())
return $this->getInput();
}
$hiddenLabel = isset($options['hiddenLabel']) ? $options['hiddenLabel'] : false;
if (empty($options['class']))

This comment has been minimized.

Copy link
@Bakual

Bakual Mar 27, 2014

Contributor

You may want to use !isset($options['class'])) here. Because currently you're going to set an empty string for an empty option, which is likely not what you want to do. 😄

This comment has been minimized.

Copy link
@chivitli

chivitli Mar 27, 2014

Author Contributor

Well, to be precise, it would make no difference, except that with empty() I get protected if someone used $options['class'] = false. :P empty() will first do !isset, which if evaluates to true it continues. Only if it evaluates to false, it checks does the value evaluate to false as well.

$options['rel'] = '';
if (empty($options['hiddenLabel']) && $this->getAttribute('hiddenLabel')) {

This comment has been minimized.

Copy link
@Bakual

Bakual Mar 27, 2014

Contributor

Please put the opening bracket { on a new line.

This comment has been minimized.

Copy link
@chivitli

chivitli Mar 27, 2014

Author Contributor

done here as well...

@wilsonge

This comment has been minimized.

Copy link
Contributor

wilsonge commented Mar 27, 2014

Looks good to me :) 1 more tester

P.S. Can you just edit the comment in the JLayout to reflect we're now passing in the options array rather than the $hiddenLabel please

@chivitli

This comment has been minimized.

Copy link
Contributor Author

chivitli commented Mar 28, 2014

Done.

@Bakual

This comment has been minimized.

Copy link
Contributor

Bakual commented Mar 28, 2014

Tested with module, plugin and template settings. Also verified that component and global config doesn't break.
Going to merge. Thanks!

@Bakual Bakual closed this in 4e432ee Mar 28, 2014

@chivitli

This comment has been minimized.

Copy link
Contributor Author

chivitli commented Mar 28, 2014

Thanks!

Bakual added a commit to Bakual/joomla-cms that referenced this pull request May 12, 2014

@piotr-cz

This comment has been minimized.

Copy link
Contributor

piotr-cz commented Apr 10, 2015

There's a problem when showon attribute is applied to field inside a group, but target field is not within it.

<fieldset>
     <field
        name="has_full_image"
        type="radio"
    >
        <option value="yes">JYES</option>
        <option value="no">JNO</option>
    </field>
</fieldset>
<fields name="images">
    <field
        name="image_fulltext"
        showon="has_full_image:yes"
    />
</fields>

because the attached rel attribute is generating id from the source field:

<div class="control-group  showon_yes"  rel="showon_jform[images][has_full_image]">
...
</div>

Dirty workaround is to place both field withing same group

@zero-24

This comment has been minimized.

Copy link
Contributor

zero-24 commented Apr 10, 2015

@piotr-cz i guess we should open a new issue for this? I think here your comment get forgotten, as this is a closed / merged PR 😄

@sovainfo

This comment has been minimized.

Copy link
Contributor

sovainfo commented Apr 10, 2015

@zero-24 Could you add it to milestone J342?

@zero-24

This comment has been minimized.

Copy link
Contributor

zero-24 commented Apr 10, 2015

@sovainfo what issue? ;) This PR gets merged into Joomla 3 and it is closed.

If we open a new Issue / PR i can add it to the milestone 😄

@sovainfo

This comment has been minimized.

Copy link
Contributor

sovainfo commented Apr 10, 2015

And if it had been assigned to the proper milestone we would be able to see when it was committed. When reporting on the milestone it would show up.

@mbabker

This comment has been minimized.

Copy link
Member

mbabker commented Apr 10, 2015

Milestones didn't start getting used until 3.3.4-ish. Long after this PR was merged.

@sovainfo

This comment has been minimized.

Copy link
Contributor

sovainfo commented Apr 10, 2015

Sorry, misread the information: 28 march 2014 instead of 2015 !

@piotr-cz

This comment has been minimized.

Copy link
Contributor

piotr-cz commented Apr 11, 2015

I've created new issue #6744.
I made a comment here, because the original author of the PR may be best person to propose optimal solution.

@chivitli

This comment has been minimized.

Copy link
Contributor Author

chivitli commented Apr 16, 2015

I think @obsidev may be the best person to suggest a solution, as (s)he is the author of the original js which works out the targets

@obsidev

This comment has been minimized.

Copy link
Contributor

obsidev commented Apr 16, 2015

Hi,

The easier will be to allow the "showon" to specify the other ID ; not complicated at all.
I was talking with @garstud during a local Joomla event and we shared some ideas on the "showon" feature, it could be a good time to implement them !

Regards,

PS : It's a "he" :) I'm also know as Jerome from the HikaShop team.

@piotr-cz

This comment has been minimized.

Copy link
Contributor

piotr-cz commented Apr 17, 2015

@obsidev I agree this is the best option but this would break B/C

@DeathMark

This comment has been minimized.

Copy link

DeathMark commented May 18, 2015

This solution doesn't work with field has multiple="true"

@Bakual

This comment has been minimized.

Copy link
Contributor

Bakual commented May 18, 2015

@DeathMark If you think this is an issue, please open a new issue on this tracker. A comment on an old closed one will only get lost in time and space.

@DeathMark

This comment has been minimized.

Copy link

DeathMark commented May 18, 2015

@Bakual Thanks. I will do it.

@stutteringp0et

This comment has been minimized.

Copy link
Contributor

stutteringp0et commented Apr 10, 2016

Has anyone thought about multiple tests? Maybe something like showon="field1:1;field2:2" - requiring multiple values to align before the field is shown?

@ggppdk

This comment has been minimized.

Copy link
Contributor

ggppdk commented Apr 10, 2016

already included in 3.5.x
see here:
#8524

showon="field1:1[AND]field2:1[AND]field3:1"
showon="field1:1[AND]field2:1[OR]field3:1"

@stutteringp0et

This comment has been minimized.

Copy link
Contributor

stutteringp0et commented Apr 10, 2016

That's awesome! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.