Skip to content

Commit

Permalink
Create possibility for nested subforms (#17552)
Browse files Browse the repository at this point in the history
* This is a combination of 8 commits.
This is the 1st commit message:

> First WIP version of supporting nested subforms.

This is the commit message #2:

> Fix nested subforms not getting correct name attribute on input fields.

This is the commit message #3:

> Fix subform rows having invalid index, fix small typo.

This is the commit message #4:

> Replace the unique subform id via random bytes by just an increasing integer in the fields type rendering process.

This is the commit message #5:

> Implement feedback from PR at Joomla;
> - Use a <template> HTML element for the template of the subform rows,
>   not a url encoded string inside of a <script> element.
> - Fix code style errors reported by phpcs.
> - Make the fixing of the unique attributes (name, id, etc) of input elements
>   of nested subform rows more errorprone, using the same method as the main
>   subform row.
> - Manually add a minified version of the javascript file.

This is the commit message #6:

> Fix failing javascript tests due to changed structure of subform repeatable template container.

This is the commit message #7:

> Change subform repeatable javascript test to correctly check on
> 0-indexed rows, previously they have been 1-indexed.

This is the commit message #8:

> Fix a problem where multi-level subforms on the same level doesnt trigger their template correctly.
> Additionally added a note why the fixScripts() method is broken and how it could get better.

* Commit code styling errors and add an aria label for the outermost 'add' button.

* Fix double group-add button on repeatable subform type.
  • Loading branch information
continga authored and Michael Babker committed Sep 22, 2018
1 parent 2b707d5 commit f2a9874
Show file tree
Hide file tree
Showing 10 changed files with 238 additions and 80 deletions.
91 changes: 58 additions & 33 deletions layouts/joomla/form/field/subform/repeatable-table.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,42 +67,67 @@
);
}
?>

<div class="row-fluid">
<div class="subform-repeatable-wrapper subform-table-layout subform-table-sublayout-<?php echo $sublayout; ?>">
<div class="subform-repeatable"
data-bt-add="a.group-add" data-bt-remove="a.group-remove" data-bt-move="a.group-move"
data-repeatable-element="tr.subform-repeatable-group"
data-rows-container="tbody.subform-repeatable-container" data-minimum="<?php echo $min; ?>" data-maximum="<?php echo $max; ?>">
<div
class="subform-repeatable"
data-bt-add="a.group-add-<?php echo $unique_subform_id; ?>"
data-bt-remove="a.group-remove-<?php echo $unique_subform_id; ?>"
data-bt-move="a.group-move-<?php echo $unique_subform_id; ?>"
data-repeatable-element="tr.subform-repeatable-group-<?php echo $unique_subform_id; ?>"
data-rows-container="tbody.rows-container-<?php echo $unique_subform_id; ?>"
data-minimum="<?php echo $min; ?>" data-maximum="<?php echo $max; ?>"
>
<table class="adminlist table table-striped table-bordered">
<thead>
<tr>
<?php echo $table_head; ?>
<?php if (!empty($buttons)) : ?>
<th style="width:8%;">
<?php if (!empty($buttons['add'])) : ?>
<div class="btn-group">
<a
class="btn btn-mini button btn-success group-add-<?php echo $unique_subform_id; ?>"
aria-label="<?php echo JText::_('JGLOBAL_FIELD_ADD'); ?>"
>
<span class="icon-plus" aria-hidden="true"></span>
</a>
</div>
<?php endif; ?>
</th>
<?php endif; ?>
</tr>
</thead>
<tbody class="rows-container-<?php echo $unique_subform_id; ?>">
<?php foreach ($forms as $k => $form):
echo $this->sublayout(
$sublayout,
array(
'form' => $form,
'basegroup' => $fieldname,
'group' => $fieldname . $k,
'buttons' => $buttons,
'unique_subform_id' => $unique_subform_id,
)
);
endforeach; ?>
</tbody>
</table>

<table class="adminlist table table-striped table-bordered">
<thead>
<tr>
<?php echo $table_head; ?>
<?php if (!empty($buttons)) : ?>
<th style="width:8%;">
<?php if (!empty($buttons['add'])) : ?>
<div class="btn-group">
<a class="group-add btn btn-mini button btn-success" aria-label="<?php echo JText::_('JGLOBAL_FIELD_ADD'); ?>"><span class="icon-plus" aria-hidden="true"></span> </a>
</div>
<?php endif; ?>
</th>
<?php endif; ?>
</tr>
</thead>
<tbody class="subform-repeatable-container">
<?php
foreach ($forms as $k => $form) :
echo $this->sublayout($sublayout, array('form' => $form, 'basegroup' => $fieldname, 'group' => $fieldname . $k, 'buttons' => $buttons));
endforeach;
?>
</tbody>
</table>
<?php if ($multiple) : ?>
<script type="text/subform-repeatable-template-section" class="subform-repeatable-template-section">
<?php echo $this->sublayout($sublayout, array('form' => $tmpl, 'basegroup' => $fieldname, 'group' => $fieldname . 'X', 'buttons' => $buttons)); ?>
</script>
<?php endif; ?>
<?php if ($multiple) : ?>
<template class="subform-repeatable-template-section"><?php echo trim(
$this->sublayout(
$sublayout,
array(
'form' => $tmpl,
'basegroup' => $fieldname,
'group' => $fieldname . 'X',
'buttons' => $buttons,
'unique_subform_id' => $unique_subform_id,
)
)
); ?></template>
<?php endif; ?>
</div>
</div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,13 @@
* @var array $buttons Array of the buttons that will be rendered
*/
extract($displayData);

?>

<tr class="subform-repeatable-group" data-base-name="<?php echo $basegroup; ?>" data-group="<?php echo $group; ?>">
<tr
class="subform-repeatable-group subform-repeatable-group-<?php echo $unique_subform_id; ?>"
data-base-name="<?php echo $basegroup; ?>"
data-group="<?php echo $group; ?>"
>
<?php foreach ($form->getFieldsets() as $fieldset) : ?>
<td class="<?php if (!empty($fieldset->class)){ echo $fieldset->class; } ?>">
<?php foreach ($form->getFieldset($fieldset->name) as $field) : ?>
Expand All @@ -32,9 +35,21 @@
<?php if (!empty($buttons)) : ?>
<td>
<div class="btn-group">
<?php if (!empty($buttons['add'])) : ?><a class="group-add btn btn-mini button btn-success" aria-label="<?php echo JText::_('JGLOBAL_FIELD_ADD'); ?>"><span class="icon-plus" aria-hidden="true"></span> </a><?php endif; ?>
<?php if (!empty($buttons['remove'])) : ?><a class="group-remove btn btn-mini button btn-danger" aria-label="<?php echo JText::_('JGLOBAL_FIELD_REMOVE'); ?>"><span class="icon-minus" aria-hidden="true"></span> </a><?php endif; ?>
<?php if (!empty($buttons['move'])) : ?><a class="group-move btn btn-mini button btn-primary" aria-label="<?php echo JText::_('JGLOBAL_FIELD_MOVE'); ?>"><span class="icon-move" aria-hidden="true"></span> </a><?php endif; ?>
<?php if (!empty($buttons['add'])) : ?>
<a class="btn btn-mini button btn-success group-add-<?php echo $unique_subform_id; ?>" aria-label="<?php echo JText::_('JGLOBAL_FIELD_ADD'); ?>">
<span class="icon-plus" aria-hidden="true"></span>
</a>
<?php endif; ?>
<?php if (!empty($buttons['remove'])) : ?>
<a class="btn btn-mini button btn-danger group-remove-<?php echo $unique_subform_id; ?>" aria-label="<?php echo JText::_('JGLOBAL_FIELD_REMOVE'); ?>">
<span class="icon-minus" aria-hidden="true"></span>
</a>
<?php endif; ?>
<?php if (!empty($buttons['move'])) : ?>
<a class="btn btn-mini button btn-primary group-move-<?php echo $unique_subform_id; ?>" aria-label="<?php echo JText::_('JGLOBAL_FIELD_MOVE'); ?>">
<span class="icon-move" aria-hidden="true"></span>
</a>
<?php endif; ?>
</div>
</td>
<?php endif; ?>
Expand Down
24 changes: 20 additions & 4 deletions layouts/joomla/form/field/subform/repeatable-table/section.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@

?>

<tr class="subform-repeatable-group" data-base-name="<?php echo $basegroup; ?>" data-group="<?php echo $group; ?>">
<tr
class="subform-repeatable-group subform-repeatable-group-<?php echo $unique_subform_id; ?>"
data-base-name="<?php echo $basegroup; ?>"
data-group="<?php echo $group; ?>"
>
<?php foreach ($form->getGroup('') as $field) : ?>
<td data-column="<?php echo strip_tags($field->label); ?>">
<?php echo $field->renderField(array('hiddenLabel' => true)); ?>
Expand All @@ -30,9 +34,21 @@
<?php if (!empty($buttons)) : ?>
<td>
<div class="btn-group">
<?php if (!empty($buttons['add'])) : ?><a class="group-add btn btn-mini button btn-success" aria-label="<?php echo JText::_('JGLOBAL_FIELD_ADD'); ?>"><span class="icon-plus" aria-hidden="true"></span> </a><?php endif; ?>
<?php if (!empty($buttons['remove'])) : ?><a class="group-remove btn btn-mini button btn-danger" aria-label="<?php echo JText::_('JGLOBAL_FIELD_REMOVE'); ?>"><span class="icon-minus" aria-hidden="true"></span> </a><?php endif; ?>
<?php if (!empty($buttons['move'])) : ?><a class="group-move btn btn-mini button btn-primary" aria-label="<?php echo JText::_('JGLOBAL_FIELD_MOVE'); ?>"><span class="icon-move" aria-hidden="true"></span> </a><?php endif; ?>
<?php if (!empty($buttons['add'])) : ?>
<a class="btn btn-mini button btn-success group-add-<?php echo $unique_subform_id; ?>" aria-label="<?php echo JText::_('JGLOBAL_FIELD_ADD'); ?>">
<span class="icon-plus" aria-hidden="true"></span>
</a>
<?php endif; ?>
<?php if (!empty($buttons['remove'])) : ?>
<a class="btn btn-mini button btn-danger group-remove-<?php echo $unique_subform_id; ?>" aria-label="<?php echo JText::_('JGLOBAL_FIELD_REMOVE'); ?>">
<span class="icon-minus" aria-hidden="true"></span>
</a>
<?php endif; ?>
<?php if (!empty($buttons['move'])) : ?>
<a class="btn btn-mini button btn-primary group-move-<?php echo $unique_subform_id; ?>" aria-label="<?php echo JText::_('JGLOBAL_FIELD_MOVE'); ?>">
<span class="icon-move" aria-hidden="true"></span>
</a>
<?php endif; ?>
</div>
</td>
<?php endif; ?>
Expand Down
39 changes: 32 additions & 7 deletions layouts/joomla/form/field/subform/repeatable.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,24 +39,49 @@
<div class="row-fluid">
<div class="subform-repeatable-wrapper subform-layout">
<div class="subform-repeatable"
data-bt-add="a.group-add" data-bt-remove="a.group-remove" data-bt-move="a.group-move"
data-repeatable-element="div.subform-repeatable-group" data-minimum="<?php echo $min; ?>" data-maximum="<?php echo $max; ?>">
data-bt-add="a.group-add-<?php echo $unique_subform_id; ?>"
data-bt-remove="a.group-remove-<?php echo $unique_subform_id; ?>"
data-bt-move="a.group-move-<?php echo $unique_subform_id; ?>"
data-repeatable-element="div.subform-repeatable-group-<?php echo $unique_subform_id; ?>"
data-minimum="<?php echo $min; ?>" data-maximum="<?php echo $max; ?>"
>

<?php if (!empty($buttons['add'])) : ?>
<div class="btn-toolbar">
<div class="btn-group">
<a class="group-add btn btn-mini button btn-success" aria-label="<?php echo JText::_('JGLOBAL_FIELD_ADD'); ?>"><span class="icon-plus" aria-hidden="true"></span> </a>
<a class="btn btn-mini button btn-success group-add-<?php echo $unique_subform_id; ?>" aria-label="<?php echo JText::_('JGLOBAL_FIELD_ADD'); ?>">
<span class="icon-plus" aria-hidden="true"></span>
</a>
</div>
</div>
<?php endif; ?>
<?php
foreach ($forms as $k => $form) :
echo $this->sublayout($sublayout, array('form' => $form, 'basegroup' => $fieldname, 'group' => $fieldname . $k, 'buttons' => $buttons));
echo $this->sublayout(
$sublayout,
array(
'form' => $form,
'basegroup' => $fieldname,
'group' => $fieldname . $k,
'buttons' => $buttons,
'unique_subform_id' => $unique_subform_id,
)
);
endforeach;
?>
<?php if ($multiple) : ?>
<script type="text/subform-repeatable-template-section" class="subform-repeatable-template-section">
<?php echo $this->sublayout($sublayout, array('form' => $tmpl, 'basegroup' => $fieldname, 'group' => $fieldname . 'X', 'buttons' => $buttons)); ?>
</script>
<template class="subform-repeatable-template-section"><?php echo trim(
$this->sublayout(
$sublayout,
array(
'form' => $tmpl,
'basegroup' => $fieldname,
'group' => $fieldname . 'X',
'buttons' => $buttons,
'unique_subform_id' => $unique_subform_id,
)
)
); ?></template>
<?php endif; ?>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,29 @@
extract($displayData);
?>

<div class="subform-repeatable-group" data-base-name="<?php echo $basegroup; ?>" data-group="<?php echo $group; ?>">
<div
class="subform-repeatable-group subform-repeatable-group-<?php echo $unique_subform_id; ?>"
data-base-name="<?php echo $basegroup; ?>"
data-group="<?php echo $group; ?>"
>
<?php if (!empty($buttons)) : ?>
<div class="btn-toolbar text-right">
<div class="btn-group">
<?php if (!empty($buttons['add'])) : ?><a class="group-add btn btn-mini button btn-success" aria-label="<?php echo JText::_('JGLOBAL_FIELD_ADD'); ?>"><span class="icon-plus" aria-hidden="true"></span> </a><?php endif; ?>
<?php if (!empty($buttons['remove'])) : ?><a class="group-remove btn btn-mini button btn-danger" aria-label="<?php echo JText::_('JGLOBAL_FIELD_REMOVE'); ?>"><span class="icon-minus" aria-hidden="true"></span> </a><?php endif; ?>
<?php if (!empty($buttons['move'])) : ?><a class="group-move btn btn-mini button btn-primary" aria-label="<?php echo JText::_('JGLOBAL_FIELD_MOVE'); ?>"><span class="icon-move" aria-hidden="true"></span> </a><?php endif; ?>
<?php if (!empty($buttons['add'])) : ?>
<a class="btn btn-mini button btn-success group-add-<?php echo $unique_subform_id; ?>" aria-label="<?php echo JText::_('JGLOBAL_FIELD_ADD'); ?>">
<span class="icon-plus" aria-hidden="true"></span>
</a>
<?php endif; ?>
<?php if (!empty($buttons['remove'])) : ?>
<a class="btn btn-mini button btn-danger group-remove-<?php echo $unique_subform_id; ?>" aria-label="<?php echo JText::_('JGLOBAL_FIELD_REMOVE'); ?>">
<span class="icon-minus" aria-hidden="true"></span>
</a>
<?php endif; ?>
<?php if (!empty($buttons['move'])) : ?>
<a class="btn btn-mini button btn-primary group-move-<?php echo $unique_subform_id; ?>" aria-label="<?php echo JText::_('JGLOBAL_FIELD_MOVE'); ?>">
<span class="icon-move" aria-hidden="true"></span>
</a>
<?php endif; ?>
</div>
</div>
<?php endif; ?>
Expand Down
24 changes: 20 additions & 4 deletions layouts/joomla/form/field/subform/repeatable/section.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,29 @@

?>

<div class="subform-repeatable-group" data-base-name="<?php echo $basegroup; ?>" data-group="<?php echo $group; ?>">
<div
class="subform-repeatable-group subform-repeatable-group-<?php echo $unique_subform_id; ?>"
data-base-name="<?php echo $basegroup; ?>"
data-group="<?php echo $group; ?>"
>
<?php if (!empty($buttons)) : ?>
<div class="btn-toolbar text-right">
<div class="btn-group">
<?php if (!empty($buttons['add'])) : ?><a class="group-add btn btn-mini button btn-success" aria-label="<?php echo JText::_('JGLOBAL_FIELD_ADD'); ?>"><span class="icon-plus" aria-hidden="true"></span> </a><?php endif; ?>
<?php if (!empty($buttons['remove'])) : ?><a class="group-remove btn btn-mini button btn-danger" aria-label="<?php echo JText::_('JGLOBAL_FIELD_REMOVE'); ?>"><span class="icon-minus" aria-hidden="true"></span> </a><?php endif; ?>
<?php if (!empty($buttons['move'])) : ?><a class="group-move btn btn-mini button btn-primary" aria-label="<?php echo JText::_('JGLOBAL_FIELD_MOVE'); ?>"><span class="icon-move" aria-hidden="true"></span> </a><?php endif; ?>
<?php if (!empty($buttons['add'])) : ?>
<a class="btn btn-mini button btn-success group-add-<?php echo $unique_subform_id; ?>" aria-label="<?php echo JText::_('JGLOBAL_FIELD_ADD'); ?>">
<span class="icon-plus" aria-hidden="true"></span>
</a>
<?php endif; ?>
<?php if (!empty($buttons['remove'])) : ?>
<a class="btn btn-mini button btn-danger group-remove-<?php echo $unique_subform_id; ?>" aria-label="<?php echo JText::_('JGLOBAL_FIELD_REMOVE'); ?>">
<span class="icon-minus" aria-hidden="true"></span>
</a>
<?php endif; ?>
<?php if (!empty($buttons['move'])) : ?>
<a class="btn btn-mini button btn-primary group-move-<?php echo $unique_subform_id; ?>" aria-label="<?php echo JText::_('JGLOBAL_FIELD_MOVE'); ?>">
<span class="icon-move" aria-hidden="true"></span>
</a>
<?php endif; ?>
</div>
</div>
<?php endif; ?>
Expand Down
10 changes: 9 additions & 1 deletion libraries/joomla/form/fields/subform.php
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ protected function getInput()
try
{
// Prepare the form template
$formname = 'subform' . ($this->group ? $this->group . '.' : '.') . $this->fieldname;
$formname = 'subform.' . str_replace(array('jform[', '[', ']'), array('', '.', ''), $control);
$tmplcontrol = !$this->multiple ? $control : $control . '[' . $this->fieldname . 'X]';
$tmpl = JForm::getInstance($formname, $this->formsource, array('control' => $tmplcontrol));

Expand Down Expand Up @@ -274,6 +274,14 @@ protected function getInput()
$data['fieldname'] = $this->fieldname;
$data['groupByFieldset'] = $this->groupByFieldset;

/**
* For each rendering process of a subform element, we want to have a
* separate unique subform id present to could distinguish the eventhandlers
* regarding adding/moving/removing rows from nested subforms from their parents.
*/
static $unique_subform_id = 0;
$data['unique_subform_id'] = ('sr-' . ($unique_subform_id++));

// Prepare renderer
$renderer = $this->getRenderer($this->layout);

Expand Down
Loading

3 comments on commit f2a9874

@okonomiyaki3000
Copy link
Contributor

@okonomiyaki3000 okonomiyaki3000 commented on f2a9874 May 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IE doesn't support the <template> element. Have we dropped support for IE?

@HLeithner
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@okonomiyaki3000 could you test this on ie and create a new issue?

@okonomiyaki3000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this was solved by #24947 but I haven't tested it.

Please sign in to comment.