Skip to content

Commit

Permalink
Fix for: Field subform (multiple) produces wrong id (joomla#16480)
Browse files Browse the repository at this point in the history 
* Fix for: Field subform (multiple) produces wrong id

* Fix subform-repeatable.js test
  • Loading branch information
@Fedik @mbabker
Fedik authored and mbabker committed Oct 15, 2017
1 parent 1d2f3fb commit 35cbb1c
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 59 deletions.
73 changes: 38 additions & 35 deletions media/system/js/subform-repeatable-uncompressed.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,52 +154,55 @@
this.lastRowNum = countnew;
$row.attr('data-group', groupnew);

// fix inputs that have a "name" attribute
var haveName = $row.find('*[name]'),
ids = {}; // collect existing id`s for fix checkboxes and radio
for(var i=0, l = haveName.length; i<l; i++){
var $el = $(haveName[i]),
name = $el.attr('name'),
id = name.replace(/(\]|\[\]$)/g, '').replace(/\[/g, '_'), // count id from name, cause we lost it after cloning
nameNew = name.replace('[' + group + '][', '['+ groupnew +']['),// count new name
idNew = id.replace(group, groupnew),// count new id
forOldAttr = id; // for fix "for" in the labels

if($el.prop('type') === 'checkbox'){// <input type="checkbox"> fix
//check if multiple
if(name.match(/\[\]$/)){
// replace a group label "for"
var groupLbl = $row.find('label[for="' + id + '"]');
if(groupLbl.length){
groupLbl.attr('for', idNew);
$el.parents('fieldset.checkboxes').attr('id', idNew);
}
// recount id
var count = ids[id] ? ids[id].length : 0;
forOldAttr = forOldAttr + count;
idNew = idNew + count;
// Fix inputs that have a "name" attribute
var haveName = $row.find('[name]'),
ids = {}; // Collect id for fix checkboxes and radio

for (var i = 0, l = haveName.length; i < l; i++) {
var $el = $(haveName[i]),
name = $el.attr('name'),
id = name.replace(/(\[\]$)/g, '').replace(/(\]\[)/g, '__').replace(/\[/g, '_').replace(/\]/g, ''), // id from name
nameNew = name.replace('[' + group + '][', '['+ groupnew +']['), // New name
idNew = id.replace(group, groupnew), // Count new id
countMulti = 0, // count for multiple radio/checkboxes
forOldAttr = id; // Fix "for" in the labels

if ($el.prop('type') === 'checkbox' && name.match(/\[\]$/)) { // <input type="checkbox" name="name[]"> fix
// Recount id
countMulti = ids[id] ? ids[id].length : 0;
if (!countMulti) {
// Set the id for fieldset and group label
$el.closest('fieldset.checkboxes').attr('id', idNew);
$row.find('label[for="' + id + '"]').attr('for', idNew).attr('id', idNew + '-lbl');
}
forOldAttr = forOldAttr + countMulti;
idNew = idNew + countMulti;
}
else if($el.prop('type') === 'radio'){// <input type="radio"> fix
// recount id
var count = ids[id] ? ids[id].length : 0;
forOldAttr = forOldAttr + count;
idNew = idNew + count;
else if ($el.prop('type') === 'radio') { // <input type="radio"> fix
// Recount id
countMulti = ids[id] ? ids[id].length : 0;
if (!countMulti) {
// Set the id for fieldset and group label
$el.closest('fieldset.radio').attr('id', idNew);
$row.find('label[for="' + id + '"]').attr('for', idNew).attr('id', idNew + '-lbl');
}
forOldAttr = forOldAttr + countMulti;
idNew = idNew + countMulti;
}

//cache ids
if(ids[id]){
// Cache already used id
if (ids[id]) {
ids[id].push(true);
} else {
ids[id] = [true];
}

// replace name to new
// Replace the name to new one
$el.attr('name', nameNew);
// set new id
// Set new id
$el.attr('id', idNew);
// guess there a label for this input
$row.find('label[for="' + forOldAttr + '"]').attr('for', idNew);
// Guess there a label for this input
$row.find('label[for="' + forOldAttr + '"]').attr('for', idNew).attr('id', idNew + '-lbl');
}
};

Expand Down
2 changes: 1 addition & 1 deletion media/system/js/subform-repeatable.js
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 8 additions & 8 deletions tests/javascript/subform-repeatable/fixtures/fixture.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@
data-rows-container="tbody" data-minimum="1" data-maximum="2">
<table>
<tbody>
<tr class="subform-repeatable-group" data-base-name="base-name" data-group="group">
<tr class="subform-repeatable-group" data-base-name="group" data-group="group0">
<td>Data 1</td>
<td>Data 2</td>
<td>
<input type="checkbox" id="original-input-checkbox" name="input">
<label for="original-input-checkbox">Checkbox label</label>
<input type="checkbox" id="jform_group__group0__checkbox" name="jform[group][group0][checkbox]">
<label for="jform_group__group0__checkbox">Checkbox label</label>
</td>
<td>
<input type="radio" id="original-input-radio" name="input">
<input type="radio" id="jform_group__group0__radio" name="jform[group][group0][radio]">
</td>
<td>
<div class="btn-group">
Expand All @@ -26,15 +26,15 @@
</tbody>
</table>
<script type="text/subform-repeatable-template-section" class="subform-repeatable-template-section">
<tr class="subform-repeatable-group" data-base-name="base-name" data-group="group">
<tr class="subform-repeatable-group" data-base-name="group" data-group="groupX">
<td>Data 3</td>
<td>Data 4</td>
<td>
<input type="checkbox" id="input_checkbox_group_test" name="input_checkbox[group][test]">
<label for="input_checkbox_group_test">Checkbox label</label>
<input type="checkbox" id="jform_group__groupX__checkbox" name="jform[group][groupX][checkbox]">
<label for="jform_group__groupX__checkbox">Checkbox label</label>
</td>
<td>
<input type="radio" id="input_radio_group_test" name="input_radio[group][test]">
<input type="radio" id="jform_group__groupX__radio0" name="jform[group][groupX][radio]">
</td>
<td>
<div class="btn-group">
Expand Down
30 changes: 15 additions & 15 deletions tests/javascript/subform-repeatable/spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,31 +39,31 @@ define(['jquery', 'testsRoot/subform-repeatable/spec-setup', 'jasmineJquery'], f
expect($container.find('tbody').children().length).toEqual(2);
});

it('Should fix the id of the template input checkbox element to "input_checkbox_base-name2_test"', function () {
expect($container.find('#input_checkbox_group_test')).not.toExist();
expect($container.find('#input_checkbox_base-name2_test')).toExist();
it('Should fix the id of the template input checkbox element to "jform_group__group2__checkbox"', function () {
expect($container.find('#jform_group__groupX__checkbox')).not.toExist();
expect($container.find('#jform_group__group2__checkbox')).toExist();
});

it('Should fix the for attribute of the checkbox label element to match the changed input id', function () {
expect($container.find('label[for="input_checkbox_base-name2_test"]')).toExist();
expect($container.find('label[for="jform_group__group2__checkbox"]')).toExist();
});

it('Should fix the name of the template input checkbox element to "input_checkbox[base-name2][test]"', function () {
expect($container.find('#input_checkbox_base-name2_test')).toHaveAttr('name','input_checkbox[base-name2][test]');
it('Should fix the name of the template input checkbox element to "jform[group][group2][checkbox]"', function () {
expect($container.find('#jform_group__group2__checkbox')).toHaveAttr('name','jform[group][group2][checkbox]');
});

it('Should fix the id of the template input radio element to "input_radio_base-name2_test0"', function () {
expect($container.find('#input_radio_group_test')).not.toExist();
expect($container.find('#input_radio_base-name2_test0')).toExist();
it('Should fix the id of the template input radio element to "jform_group__group2__radio0"', function () {
expect($container.find('#jform_group__groupX__radio0')).not.toExist();
expect($container.find('#jform_group__group2__radio0')).toExist();
});

it('Should fix the name of the template input radio element to "input_radio[base-name2][test]"', function () {
expect($container.find('#input_radio_base-name2_test0')).toHaveAttr('name','input_radio[base-name2][test]');
it('Should fix the name of the template input radio element to "jform[group][group2][radio]"', function () {
expect($container.find('#jform_group__group2__radio0')).toHaveAttr('name','jform[group][group2][radio]');
});

it('Should have captured the template correctly', function () {
var $newElement = $container.find('tbody').children().last();

expect($newElement).toContainText('Data 3');
expect($newElement).toContainText('Data 4');
expect($newElement).toContainText('Checkbox label');
Expand All @@ -76,8 +76,8 @@ define(['jquery', 'testsRoot/subform-repeatable/spec-setup', 'jasmineJquery'], f
expect($container.find('tbody').children().last()).toHaveAttr('data-new', 'true');
});

it('Should set data-group attribute to "base-name2" in the new element', function () {
expect($container.find('tbody').children().last()).toHaveAttr('data-group', 'base-name2');
it('Should set data-group attribute to "group2" in the new element', function () {
expect($container.find('tbody').children().last()).toHaveAttr('data-group', 'group2');
});

it('Should trigger subform-row-add event', function () {
Expand All @@ -88,7 +88,7 @@ define(['jquery', 'testsRoot/subform-repeatable/spec-setup', 'jasmineJquery'], f
describe('Remove existing row', function () {
beforeAll(function () {
$container.find('a.group-remove.generated').click();
$container.find('subform-original-remove').click();
$container.find('#subform-original-remove').click();
});

it('Should remove the added row from the table', function () {
Expand Down

0 comments on commit 35cbb1c

Please sign in to comment.