Skip to content

Commit c449f1f

Browse files
author
Rene Pasing
committed
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.
1 parent 09499ed commit c449f1f

File tree

5 files changed

+110
-111
lines changed

5 files changed

+110
-111
lines changed

layouts/joomla/form/field/subform/repeatable-table.php

Lines changed: 44 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -67,66 +67,64 @@
6767
);
6868
}
6969
?>
70-
7170
<div class="row-fluid">
7271
<div class="subform-repeatable-wrapper subform-table-layout subform-table-sublayout-<?php echo $sublayout; ?>">
73-
<div class="subform-repeatable"
72+
<div
73+
class="subform-repeatable"
7474
data-bt-add="a.group-add-<?php echo $unique_subform_id; ?>"
7575
data-bt-remove="a.group-remove-<?php echo $unique_subform_id; ?>"
7676
data-bt-move="a.group-move-<?php echo $unique_subform_id; ?>"
7777
data-repeatable-element="tr.subform-repeatable-group-<?php echo $unique_subform_id; ?>"
7878
data-rows-container="tbody.rows-container-<?php echo $unique_subform_id; ?>"
79-
data-minimum="<?php echo $min; ?>" data-maximum="<?php echo $max; ?>">
79+
data-minimum="<?php echo $min; ?>" data-maximum="<?php echo $max; ?>"
80+
>
81+
<table class="adminlist table table-striped table-bordered">
82+
<thead>
83+
<tr>
84+
<?php echo $table_head; ?>
85+
<?php if (!empty($buttons)) : ?>
86+
<th style="width:8%;">
87+
<?php if (!empty($buttons['add'])) : ?>
88+
<div class="btn-group">
89+
<a class="btn btn-mini button btn-success group-add-<?php echo $unique_subform_id; ?>">
90+
<span class="icon-plus"></span>
91+
</a>
92+
</div>
93+
<?php endif; ?>
94+
</th>
95+
<?php endif; ?>
96+
</tr>
97+
</thead>
98+
<tbody class="rows-container-<?php echo $unique_subform_id; ?>">
99+
<?php foreach ($forms as $k => $form):
100+
echo $this->sublayout(
101+
$sublayout,
102+
array(
103+
'form' => $form,
104+
'basegroup' => $fieldname,
105+
'group' => $fieldname . $k,
106+
'buttons' => $buttons,
107+
'unique_subform_id' => $unique_subform_id,
108+
)
109+
);
110+
endforeach; ?>
111+
</tbody>
112+
</table>
80113

81-
<table class="adminlist table table-striped table-bordered">
82-
<thead>
83-
<tr>
84-
<?php echo $table_head; ?>
85-
<?php if (!empty($buttons)) : ?>
86-
<th style="width:8%;">
87-
<?php if (!empty($buttons['add'])) : ?>
88-
<div class="btn-group">
89-
<a class="btn btn-mini button btn-success group-add-<?php echo $unique_subform_id; ?>">
90-
<span class="icon-plus"></span>
91-
</a>
92-
</div>
93-
<?php endif; ?>
94-
</th>
95-
<?php endif; ?>
96-
</tr>
97-
</thead>
98-
<tbody class="rows-container-<?php echo $unique_subform_id; ?>">
99-
<?php foreach ($forms as $k => $form):
100-
echo $this->sublayout(
114+
<?php if ($multiple) : ?>
115+
<template class="subform-repeatable-template-section"><?php echo trim(
116+
$this->sublayout(
101117
$sublayout,
102118
array(
103-
'form' => $form,
119+
'form' => $tmpl,
104120
'basegroup' => $fieldname,
105-
'group' => $fieldname . $k,
121+
'group' => $fieldname . 'X',
106122
'buttons' => $buttons,
107123
'unique_subform_id' => $unique_subform_id,
108124
)
109-
);
110-
endforeach; ?>
111-
</tbody>
112-
</table>
113-
<?php if ($multiple) : ?>
114-
<script type="text/subform-repeatable-template-section" class="subform-repeatable-template-section">
115-
<?php /** Do a rawurlencode to not tamper with HTML elements, especially with
116-
* nested subforms (subform in subform), this might contain a
117-
* </script> tag, which else blows up our markup. */ ?>
118-
<?php echo rawurlencode(trim($this->sublayout(
119-
$sublayout,
120-
array(
121-
'form' => $tmpl,
122-
'basegroup' => $fieldname,
123-
'group' => $fieldname . 'X',
124-
'buttons' => $buttons,
125-
'unique_subform_id' => $unique_subform_id,
126-
)
127-
))); ?>
128-
</script>
129-
<?php endif; ?>
125+
)
126+
); ?></template>
127+
<?php endif; ?>
130128
</div>
131129
</div>
132130
</div>

layouts/joomla/form/field/subform/repeatable.php

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@
4343
data-bt-remove="a.group-remove-<?php echo $unique_subform_id; ?>"
4444
data-bt-move="a.group-move-<?php echo $unique_subform_id; ?>"
4545
data-repeatable-element="div.subform-repeatable-group-<?php echo $unique_subform_id; ?>"
46-
data-minimum="<?php echo $min; ?>" data-maximum="<?php echo $max; ?>">
46+
data-minimum="<?php echo $min; ?>" data-maximum="<?php echo $max; ?>"
47+
>
4748

4849
<?php if (!empty($buttons['add'])) : ?>
4950
<div class="btn-toolbar">
@@ -69,21 +70,18 @@
6970
endforeach;
7071
?>
7172
<?php if ($multiple) : ?>
72-
<script type="text/subform-repeatable-template-section" class="subform-repeatable-template-section">
73-
<?php /** Do a rawurlencode to not tamper with HTML elements, especially with
74-
* nested subforms (subform in subform), this might contain a
75-
* </script> tag, which else blows up our markup. */ ?>
76-
<?php echo rawurlencode(trim($this->sublayout(
77-
$sublayout,
78-
array(
79-
'form' => $tmpl,
80-
'basegroup' => $fieldname,
81-
'group' => $fieldname . 'X',
82-
'buttons' => $buttons,
83-
'unique_subform_id' => $unique_subform_id,
73+
<template class="subform-repeatable-template-section"><?php echo trim(
74+
$this->sublayout(
75+
$sublayout,
76+
array(
77+
'form' => $tmpl,
78+
'basegroup' => $fieldname,
79+
'group' => $fieldname . 'X',
80+
'buttons' => $buttons,
81+
'unique_subform_id' => $unique_subform_id,
82+
)
8483
)
85-
))); ?>
86-
</script>
84+
); ?></template>
8785
<?php endif; ?>
8886
</div>
8987
</div>

libraries/joomla/form/fields/subform.php

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -274,11 +274,13 @@ protected function getInput()
274274
$data['fieldname'] = $this->fieldname;
275275
$data['groupByFieldset'] = $this->groupByFieldset;
276276

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

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

media/system/js/subform-repeatable-uncompressed.js

Lines changed: 25 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,8 @@
6868
$.subformRepeatable.prototype.prepareTemplate = function(){
6969
// create from template
7070
if (this.options.rowTemplateSelector) {
71-
var tmplElement = this.$container.find(this.options.rowTemplateSelector).last()[0] || {};
72-
// do a decodeURIComponent() here, because the text value is url encoded
73-
// to make sure we dont destroy our markup
74-
this.template = decodeURIComponent($.trim(tmplElement.text || tmplElement.textContent)); //(text || textContent) is IE8 fix
71+
// Find the template element and get its HTML content, this is our template.
72+
this.template = $.trim(this.$container.find(this.options.rowTemplateSelector).html()) || '';
7573
}
7674
// create from existing rows
7775
else {
@@ -144,13 +142,18 @@
144142
$row.remove();
145143
};
146144

147-
// fix names ind id`s for field that in $row
148-
$.subformRepeatable.prototype.fixUniqueAttributes = function($row, count){
149-
var group = $row.attr('data-group'),// current group name
150-
basename = $row.attr('data-base-name'), // group base name, without count
151-
count = count || 0,
145+
// fix names and id`s for fields in $row
146+
$.subformRepeatable.prototype.fixUniqueAttributes = function(
147+
$row, // the jQuery object to do fixes in
148+
count, // existing count of rows
149+
group, // current group name, e.g. 'optionsX'
150+
basename // group base name, without count, e.g. 'options'
151+
) {
152+
var group = (typeof group === 'undefined' ? $row.attr('data-group') : group),
153+
basename = (typeof basename === 'undefined' ? $row.attr('data-base-name') : basename),
154+
count = (typeof count === 'undefined' ? 0 : count),
152155
countnew = Math.max(this.lastRowNum, count),
153-
groupnew = basename + countnew; // new group name
156+
groupnew = basename + countnew;
154157

155158
this.lastRowNum = countnew;
156159
$row.attr('data-group', groupnew);
@@ -203,43 +206,20 @@
203206
$row.find('label[for="' + forOldAttr + '"]').attr('for', idNew);
204207
}
205208

206-
// Create 2 strings: basename + old group, and basename + new group
207-
var search = '[' + basename + '][' + group + ']',
208-
replace = '[' + basename + '][' + groupnew + ']';
209-
// Does our row still contain the basename + old group? This should not happen!
210-
if ($row.html().indexOf(search) !== -1) {
211-
console.log('Old basename+group still existant in $row html');
212-
}
213-
// Recursively replace our basename + old group with basename + new group
214-
// inside of nested subform template elements.
215-
this.recursiveReplaceNested($row, search, replace);
216-
};
217-
218-
$.subformRepeatable.prototype.recursiveReplaceNested = function($row, search, replace) {
219-
// Try to find the row remplate selector in $row
209+
/**
210+
* Recursively replace our basename + old group with basename + new group
211+
* inside of nested subform template elements. First we try to find such
212+
* template elements, then we iterate through them and do the same replacements
213+
* that we have made here inside of them.
214+
*/
220215
var nestedTemplates = $row.find(this.options.rowTemplateSelector);
221-
if (nestedTemplates.length < 1) {
222-
return;
223-
}
224216
// If we found it, iterate over the found ones (might be more than one!)
225217
for (var i = 0; i < nestedTemplates.length; i++) {
226-
// Get the element
227-
var nestedTemplate = $(nestedTemplates[i]);
228-
// Do our replacement; the HTML content is urlencoded, so we urlencode
229-
// our search/replace parameters, also because we then don't need to
230-
// regexp escape the search parameter (we are using regexp to globally replace).
231-
nestedTemplate.html(nestedTemplate.html().replace(
232-
new RegExp(encodeURIComponent(search), 'g'),
233-
encodeURIComponent(replace)
234-
));
235-
// URI decode its HTML content to could have access to deeper nested ones
236-
var nestedTemplateContent = decodeURIComponent(nestedTemplate.html());
237-
// Create a new element out of it
238-
var nestedElement = $(nestedTemplateContent);
239-
// And recursively do the replacements more times
240-
this.recursiveReplaceNested(nestedElement, search, replace);
241-
// Now re-insert the replaced html content into our element
242-
nestedTemplate.html(encodeURIComponent(nestedElement.prop('outerHTML')));
218+
// Get the nested templates content (as DocumentFragment) and cast it
219+
// to a jQuery object
220+
var nestedTemplate = $($(nestedTemplates[i]).prop('content'));
221+
// Fix the attributes for this nested template.
222+
this.fixUniqueAttributes(nestedTemplate, count, group, basename);
243223
}
244224
};
245225

@@ -312,7 +292,7 @@
312292
repeatableElement: ".subform-repeatable-group",
313293
// selector for the row template element with URL-encoded template inside it,
314294
// must *NOT* be unique per nested subform!
315-
rowTemplateSelector: 'script.subform-repeatable-template-section',
295+
rowTemplateSelector: 'template.subform-repeatable-template-section',
316296
// container for rows, same as main container by default
317297
rowsContainer: null
318298
};

0 commit comments

Comments
 (0)