Fabrik / fabrik

Fabrik for Joomla 3.x
http://fabrikar.com
Other
254 stars 380 forks source link

Element JavaScript Events Affecting Other Similarly-Named Elements #1131

Open njbair opened 10 years ago

njbair commented 10 years ago

Let's say I have a form with two groups. Group A contains a checkbox configured to conditionally show/hide Group B based on its own value. Group B contains other checkboxes.

Form Structure

The form structure is set up like so:

Group A
  Poetry { name: 'poetry' }
    [ ] Show Group B { 'foo' }

Group B
  Lines { name: 'poetry_lines' }
    [ ] Roses are red { 'red' }
    [ ] Violets are blue { 'blue' }
    [ ] Sugar is sweet { 'sweet' }
    [ ] ...and Bar is foo { 'foo' }

JavaScript Rules

The checkbox named poetry contains the following JavaScript rules:

Toggling the _poetry_ checkbox should show Group B when checked, and hide Group B when unchecked. None of the items under Group B should have any effect on the group's visibility.

Actual Behavior

Toggling the poetry checkbox works as expected. But when you toggle the items under poetry_lines, all of Group B disappears.

The only exception is the fourth _poetry_lines_ checkbox, whose value ('foo') is the same as that of the _poetry_ checkbox. When the first three boxes are unchecked and you mark the fourth box checked, Group B does not disappear.

Analysis

It appears that Fabrik is assigning the JavaScript event listeners to all elements whose name contains poetry. This is likely being caused by the use of a DOM selector method which returns a set of results instead of a single item.

Solution

Change the way Fabrik selects the element when assigning event listeners by using a selector method which returns an exact match only.

I would be glad to patch this myself but I've been through the Fabrik source and I've determined that I simply don't know enough about the Fabrik codebase or the MooTools library it uses.

Workaround

Don't use element names that could return multiple results. In this example, poetry could be renamed poetry_toggle or poetry_show.

Sophist-UK commented 10 years ago

If this is indeed using a selector which can select multiple results, then this is definitely a bug because the dropdown to select an element or Group specifically targets a single item. I will try to take a look at it later today.

Sophist-UK commented 10 years ago

@njbair: Can you please look at the page created in your example and post the lines from the HTML which contain the dispatchEvent calls.

njbair commented 10 years ago

Here's the entire script block:

<script type="text/javascript">
requirejs.config({
    baseUrl: 'https://isee.bad-dev.com/',
    paths: {
        fab : 'media/com_fabrik/js',
        element : 'plugins/fabrik_element',
        list : 'plugins/fabrik_list',
        form : 'plugins/fabrik_form',
        cron : 'plugins/fabrik_cron',
        viz : 'plugins/fabrik_visualization',
        admin : 'administrator/components/com_fabrik/views',
        adminfields : 'administrator/components/com_fabrik/models/fields'},
    shim: {"fab\/fabrik-min":{"deps":["fab\/mootools-ext-min","fab\/lib\/Event.mock","fab\/tipsBootStrapMock-min","fab\/encoder-min"]},"fab\/window-min":{"deps":["fab\/fabrik-min"]},"fab\/elementlist-min":{"deps":["fab\/fabrik-min","fab\/element-min"]},"fabrik\/form-min":{"deps":["fab\/element-min","lib\/form_placeholder\/Form.Placeholder","fab\/encoder-min"]},"element\/internalid\/internalid-min":{"deps":["fab\/element-min"]},"element\/date\/date-min":{"deps":["fab\/element-min"]},"element\/checkbox\/checkbox-min":{"deps":["fab\/element-min","fab\/elementlist-min"]}},
    waitSeconds: 30,
});

requirejs(['fab/fabrik-min', 'fab/tipsBootStrapMock-min'], function () {
    Fabrik.liveSite = 'https://isee.bad-dev.com/';
    Fabrik.debug = false;
    Fabrik.bootstrapped = true;
    Fabrik.tips = new FloatingTips('.fabrikTip', {"tipfx":"Fx.Transitions.linear.easeIn","duration":"500","distance":50,"fadein":false});
    Fabrik.addEvent('fabrik.list.updaterows', function () {
        // Reattach new tips after list redraw
        Fabrik.tips.attach('.fabrikTip');
    });
    Fabrik.addEvent('fabrik.plugin.inlineedit.editing', function () {
        Fabrik.tips.hideAll();
    });
    Fabrik.addEvent('fabrik.list.inlineedit.setData', function () {
        Fabrik.tips.attach('.fabrikTip');
    });
});

window.addEvent('fabrik.loaded', function() {
  $$('a.fabrikWin').each(function(el, i) {
    el.addEvent('click', function(e) {
        var opts = {"id":"fabwin","title":"Advanced search","loadMethod":"xhr","minimizable":false,"collapsible":true,"width":500,"height":150};
        e.stop();
      opts2 = JSON.decode(el.get('rel'));
      opts = Object.merge(opts, opts2 || {});
      opts.contentURL = el.href;
      if (opts.id === 'fabwin') {
        opts.id += i;
      }
      Fabrik.getWindow(opts);
    });
  });
});
requirejs(['fab/fabrik-min', 'fab/window-min', 'fab/lib/form_placeholder/Form.Placeholder', 'fab/form-min', 'fab/form-submit-min', 'fab/element-min', 'element/internalid/internalid-min', 'fab/elementlist-min', 'element/date/date-min', 'element/checkbox/checkbox-min'], function () {
        var form_17 = Fabrik.form('form_17', 17, {"admin":false,"ajax":false,"ajaxValidation":false,"primaryKey":"isee_bug_test___id","error":"Some parts of your form have not been correctly filled in","pages":{"0":[64,65]},"plugins":[],"multipage_save":0,"editable":true,"start_page":0,"inlineMessage":false,"rowid":"","listid":17,"images":{"alert":"icon-warning ","action_check":"icon-action_check ","ajax_loader":"<i class=\"icon-spinner icon-spin\"><\/i>"},"fabrik_window_id":"","submitOnEnter":false,"hiddenGroup":{"64":false,"65":false},"maxRepeat":{"64":0,"65":0},"minRepeat":{"64":1,"65":1},"showMaxRepeats":{"64":false,"65":false},"join_group_ids":[],"group_repeats":[],"group_joins_ids":[]});
    form_17.addElements(
{"64":[["FbInternalId","isee_bug_test___id",{"repeatCounter":0,"editable":true,"value":"","label":"id","defaultVal":"","inRepeatGroup":false,"fullName":"isee_bug_test___id","watchElements":[],"canRepeat":false,"isGroupJoin":false,"validations":false,"joinid":0}],["FbDateTime","isee_bug_test___date_time",{"repeatCounter":0,"editable":true,"value":"2013-12-28 15:02:13","label":"date time","defaultVal":"2013-12-28 10:02:13","inRepeatGroup":false,"fullName":"isee_bug_test___date_time","watchElements":[],"canRepeat":false,"isGroupJoin":false,"validations":false,"joinid":0,"hidden":true,"showtime":false,"timelabel":"time","typing":true,"timedisplay":1,"dateTimeFormat":"H:i","calendarSetup":{"inputField":"isee_bug_test___date_time","button":"isee_bug_test___date_time_cal_img","align":"Tl","singleClick":true,"firstDay":0,"ifFormat":"%Y-%m-%d %H:%M:%S","timeFormat":24,"dateAllowFunc":null},"advanced":false}],["FbCheckBox","isee_bug_test___poetry",{"repeatCounter":0,"editable":true,"value":[""],"label":"","defaultVal":"","inRepeatGroup":false,"fullName":"isee_bug_test___poetry","watchElements":[],"canRepeat":false,"isGroupJoin":false,"validations":false,"joinid":0,"data":{"foo":"Show Group B"},"allowadd":false}]],"65":[["FbCheckBox","isee_bug_test___poetry_lines",{"repeatCounter":0,"editable":true,"value":[""],"label":"","defaultVal":"","inRepeatGroup":false,"fullName":"isee_bug_test___poetry_lines","watchElements":[],"canRepeat":false,"isGroupJoin":false,"validations":false,"joinid":0,"data":{"red":"Roses are red","blue":"Violets are blue","sweet":"Sugar is sweet","foo":"...and Bar is foo"},"allowadd":false}]]}
    );
Fabrik.blocks['form_17'].addElementFX('fabrik_trigger_group_group65', 'hide');
Fabrik.blocks['form_17'].dispatchEvent('checkbox', 'isee_bug_test___poetry', 'load', 'if (this.get(\'value\') != \'foo\') {Fabrik.blocks[\'form_17\'].doElementFX(\'fabrik_trigger_group_group65\', \'hide\', this)}');
Fabrik.blocks['form_17'].dispatchEvent('checkbox', 'isee_bug_test___poetry', 'change', 'if (this.get(\'value\') != \'foo\') {Fabrik.blocks[\'form_17\'].doElementFX(\'fabrik_trigger_group_group65\', \'hide\', this)}');
Fabrik.blocks['form_17'].dispatchEvent('checkbox', 'isee_bug_test___poetry', 'change', 'if (this.get(\'value\') == \'foo\') {Fabrik.blocks[\'form_17\'].doElementFX(\'fabrik_trigger_group_group65\', \'show\', this)}');

    new Form.Placeholder('.fabrikForm input');
    function submit_form() {
    return false;
}
function submitbutton(button) {
    if (button=="cancel") {
        document.location = '/fabrik-bug-test/viewTable?cid=17';
    }
    if (button == "cancelShowForm") {
        return false;
    }
}
});

</script>
Sophist-UK commented 10 years ago

I think I have narrowed this down to a bug in ElementList.js addNewEvent - will diagnose further and see if I can fix this.

Sophist-UK commented 10 years ago

1133 fixes this issue.