picklist repeat counter not being incrimented

[Bauer]

I'm using a picklist element in a repeat group of a form.
When a new repeat group is added the element's id is not being initialized correctly. The id is the exact clone of the 1st group's id (i.e. the repeat counter always stays at zero.) The id's for all the other elements in this repeat group are incrementing as expected.

Can someone please confirm this as a bug? I'd really like to use this picklist element, but it's a no-go as is.

 

[cheesegrits]

I had a hack at it last night, but only got about half way. I can see the problem, but not entirely sure of the best solution, if the cloned() method needs to just rebuild the inner div's and li's, or if it needs it's own cloneUpdateIds() method, or both.

I'll talk to The Boss on Monday.

-- hugh

 

[Bauer]

Thanks Hugh. I've got a few dozen hours into farting around with this myself since I first discovered the problem and started this support thread.

There isn't much to the picklist element code - but that still doesn't mean I understand everything that's being done.

I did discover that, by adding the 'fabrikinput' class, the UL and LI tags the RepeatCounter index in any added repeat group will be incremented correctly. But then the LI tags don't use the special id created by the picklist element that includes the '_value_thevalue' as part of the id.

Also, in the actual hidden input element that holds the 'toList' values (i.e. the values that get stored to the database table), it still clones the values from the first repeat group to the added repeat group rather than resetting the fromList and toList to the default (all items moved to the fromList). BTW, that hidden input tag was the only part of the picklist element that was incrementing the id with the current/correct RepeatCounter.

But I'm using some javascript in the form_xx.js file to change other elements in the same repeat group form in real-time with the assistance of the userAjax.js feature of fabrik. (Cool - you discover something new every day). In some instances I don't want the picklist element shown at all. So I need the IDs of that picklist element to represent the correct repeat group.

I'm not sure if the issues are with the element or that the fabrik core code is not recognizing the way the picklist elements creates those IDs. I don't think you'd want to change the core fabrik form or element model code just because this one element isn't playing by the expected rules. And I see no point in writing my own javascript in the form_xx.js file to handle it. So I was trying to play the hero and just change the picklist php and js code for you. But I'm having problems figuring that all out. How about storing that 'value_thevalue' part of the UL/LI id in a different attribute other than the ID itself? (This is what makes the UL and LI id's so different from other Fabrik elements.) Like maybe the title attribute - or as part of an additional class name like 'value_thevalue'? This way code can be changed in the picklist.php and picklist.js files (to find the value of the LI items in another way) without having to change anything else in fabrik.

As usual, just my 2 cents - and probably all it's worth.
Thanks again, Hugh - and please keep me abreast of any progress. This is the final piece of the puzzle in a 3-group tabbed form that is the most complex part of what seems to be a neverending project.

 

[cheesegrits]

Well, that's pretty much where I was at, and what I summarized in my first response. I'm not entirely sure if this needs to be dealt with on a generic basis in the main element model, or (more likely) in an override if one of the element model methods in the picklist model ... and if so, which of the two methods I named would be the one to do it in. I'm also a little unclear on whether the picklist is considered to have "subOptions", in the sense that the existing models interpret that.

Just FYI, it can't happen in the PHP, as that never gets invoked when a group is repeated. And the JS methods I mentioned would "know" what the repeat number is, as the form model stuffs that somewhere in the element object for that purpose. It's just a case of exactly where the code needs to go that re-ID's the picklist structure. I *think* it's going to have to go in the cloned() method, because that's where we destroy and recreate the actual Mootools "sortable", and pass it the two div's ('from' and 'to') with the lists in them. So i think it needs to happen between the destroy, and the makeSortable(), or whatever that function is called.

As I said, I'll talk to Rob tomorrow about it.

-- hugh

 

[Bauer]

I got this working by just eliminating the ID property in the UL and LI altogether. Why are they even needed really?

In my form_xx.js code I can still get the correct UL list and LI values, even without having an LI id, by using the closest('.fromList') or closest('.toList') function to get the correct UL lists for the element in the current repeat group - in relation to that hidden input tag, which contains the picklist element's value property that is submitted with the form (the actual picked 'toList' values) - and DOES contain the correct id (and name) attributes after a new repeat group is added.

What I basically did was change picklist.php to add the LI 'value' (that was being appended to the LI id (i.e. '_value_thisvalue') to the class attribute - and changed the picklist.js code so as to get that value from the class attribute of the LI rather than the id. And... also changed the js code used to create the LI for any new user-added value that might get added - using the same attributes that were used in the php code; i.e. Don't add an id, and insert the value into the class. (I haven't tested this part as I don't allow adding to the picklist element in my form - but the changed code should work.)

Here are the changes...
In picklist.php...
In public function render - Changed the section that renders the UL and LI tags...
From...

$fromlist[] = FText::_('PLG_FABRIK_PICKLIST_FROM') . ':<ul id="' . $id . '_fromlist" class="picklist well well-small fromList">';
$tolist[] = FText::_('PLG_FABRIK_PICKLIST_TO') . ':<ul id="' . $id . '_tolist" class="picklist well well-small toList">';
foreach ($arVals as $v)
{
    if (!in_array($v, $arSelected))
    {
          $fromlist[] = '<li id="' . $id . '_value_' . $v . '" class="picklist">' . $arTxt[$i] . '</li>';
    }
    $i++;
}
$i = 0;
$lookup = array_flip($arVals);
foreach ($arSelected as $v)
{
    if ($v == '' || $v == '-' || $v == '[""]')
    {
          continue;
    }
    $k = JArrayHelper::getValue($lookup, $v);
    $tmptxt = addslashes(htmlspecialchars(JArrayHelper::getValue($arTxt, $k)));
    $tolist[] = '<li id="' . $id . '_value_' . $v . '" class="' . $v . '">' . $tmptxt . '</li>';
    $aRoValues[] = $tmptxt;
    $i++;
}

To...

$fromlist[] = FText::_('PLG_FABRIK_PICKLIST_FROM') . ':<ul class="picklist well well-small fromList">';
$tolist[] = FText::_('PLG_FABRIK_PICKLIST_TO') . ':<ul class="picklist well well-small toList">';
foreach ($arVals as $v)
{
    if (!in_array($v, $arSelected))
    {
          $fromlist[] = '<li class="picklist ' . $v . '">' . $arTxt[$i] . '</li>';
    }
    $i++;
}
$i = 0;
$lookup = array_flip($arVals);
foreach ($arSelected as $v)
{
    if ($v == '' || $v == '-' || $v == '[""]')
    {
          continue;
    }
    $k = JArrayHelper::getValue($lookup, $v);
    $tmptxt = addslashes(htmlspecialchars(JArrayHelper::getValue($arTxt, $k)));
    $tolist[] = '<li class="picklist ' . $v . '">' . $tmptxt . '</li>';
    $aRoValues[] = $tmptxt;
    $i++;
}

Then in picklist.js...
In setData: function...
Changed...

return item.id
    .replace(this.options.element + '_value_', '');

To...

return (item.className.indexOf('picklist') > -1) ? item.className.substr(item.className.indexOf('picklist')+9) : "";

in watchAdd: function
Changed...

var li = new Element('li', {
          'class' : 'picklist',
          'id' : this.element.id
      }).set('text', label);

To...

var li = new Element('li', {
          'class' : 'picklist ' + val
    }).set('text', label);

It seems to be working as expected now.

 

[cheesegrits]

I expect we added ID's as it Seemed Like a Good Idea At the Time. When we build stuff like this, we kind of work to a formula, usually based on some similar(ish) element design, and most elements which have sub-structures typically need ID's at some point, for doing something with. And in this case, when we built the picklist, it was for a specific user, for a specific project, on a specific budget, who had no requirement for repeating.

The repeat stuff got added at a later date, then we changed some stuff about how repeating works, so bitrot obviously set in on this element. And you happen to be the first person that has tried to use a picklist in a repeat group since then and discovered the mold in the innards. Lucky you.

I'll have a hack at merging your changes when I get some time. Thanks for your efforts on this.

-- hugh

 

[Bauer]

No problem, I understand how code is often 'dated'. So thanks in advance for putting that on your 'to do ' list.

I have another quick fix for the Date element that I'm hoping for - and just about to post.