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
Closed
Diff settings

Always

Just for now

@@ -19,9 +19,17 @@
?>

<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

👍

{
JHtml::_('jquery.framework');
JHtml::_('script', 'jui/cms.js', false, true);
}
?>

<div class="control-group <?php echo $displayData['options']['class']; ?>" <?php echo $displayData['options']['rel']; ?>>
<?php if (empty($displayData['options']['hiddenLabel'])) : ?>
<div class="control-label"><?php echo $displayData['label']; ?></div>
<?php endif; ?>
<div class="controls"><?php echo $displayData['input']; ?></div>
</div>
</div>
@@ -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['class'] = '';
}
$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...

$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

$showon = explode(':', $showon, 2);
$options['class'] .= ' showon_' . implode(' showon_', explode(',', $showon[1]));
$id = $this->getName($showon[0]);
$options['rel'] = ' rel="showon_' . $id . '"';
$options['showonEnabled'] = true;
}
return JLayoutHelper::render($this->renderLayout, array('input' => $this->getInput(), 'label' => $this->getLabel(), 'hiddenLabel' => $hiddenLabel));
return JLayoutHelper::render($this->renderLayout, array('input' => $this->getInput(), 'label' => $this->getLabel(), 'options' => $options));
}
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.