[Zope3-dev] OrderedMultiSelectWidget problem reloaded

Dominik Huber dominik.huber at perse.ch
Thu Nov 23 17:53:42 EST 2006


Dear form-framework experts/users,

Yesterday I applied the first time an editform to a schema containing a
'List' field of value_type 'Choice'.
The OrderedMultiSelectWidget is invoked by default. At the first glance
it works fine but by and by I discovered the following bugs/missfeatures:

- Does not invoke adapted contexts correctly (source versus context,
general problem of .utility.setUpEditWidgets())

- Does not take the _type attribute of the field into account (->
.browser.itemwidgets.MultiDataHelper,
.browser.sequencewidget.SequenceWidget)

- Does not resolve vocabularies correctly in case of InputErrors
(probably -> overwrite _getFormValue of
.browser.itemwidgets.ItemsMultiEditWidgetBase)

- Does not differ choices in relation to the unique property of the
field (-> .browser.itemwidgets.OrderedMultiSelectWidget)
- Does not handle missing_value of sequences (->
.browser.itemwidgets.OrderedMultiSelectWidget)

- Does not respect the min_length and max_length property of the
underlying field like the sequence widget (-> disable buttons)

Afterwards I have seen that Adam reported already
OrderedMultiSelectWidget problems
(http://www.zope.org/Collectors/Zope3-dev/451).
This issue was deferred by Jim. I spent a few hour to track down the
problems on Zope 3.3.0 and produced the attached patch.

Are people interested to resume this issue or not? Are there any
objections in relation to the proposed patch?

Regards,
Dominik

Index: browser/itemswidgets.py
===================================================================
--- browser/itemswidgets.py    (revision 71100)
+++ browser/itemswidgets.py    (working copy)
@@ -28,6 +28,7 @@
 from zope.app.form.browser.widget import SimpleInputWidget, renderElement
 from zope.app.form.interfaces import IInputWidget, IDisplayWidget
 from zope.app.form.interfaces import ConversionError
+from zope.app.form.interfaces import InputErrors
 from zope.app.pagetemplate.viewpagetemplatefile import ViewPageTemplateFile
 
 from zope.app.i18n import ZopeMessageFactory as _
@@ -144,7 +145,6 @@
             "It may be inherited from the mix-in classes 
SingleDataHelper\n"
             "or MultiDataHelper")
 
-
 class SingleDataHelper(object):
     """Mix-in helper class for getting the term from the HTML form.
 
@@ -190,13 +190,14 @@
     def _toFieldValue(self, input):
         """See SimpleInputWidget"""
         if input is None:
-            return []
-        if not isinstance(input, list):
-            input = [input]
-        try:
-            values = self.convertTokensToValues(input)
-        except InvalidValue, e:
-            raise ConversionError("Invalid value", e)
+            values = []
+        else:
+            if not isinstance(input, list):
+                input = [input]
+            try:
+                values = self.convertTokensToValues(input)
+            except InvalidValue, e:
+                raise ConversionError("Invalid value", e)
 
         # All AbstractCollection fields have a `_type` attribute specifying
         # the type of collection. Use it to generate the correct type,
@@ -209,7 +210,6 @@
         else:
             return values
 
-
     def _getDefault(self):
         # Return the default value for this widget;
         # may be overridden by subclasses.
@@ -487,6 +487,36 @@
                         "(no values)")
     _displayItemForMissingValue = False
 
+    def _getFormValue(self):
+        """Returns a value suitable for use in an HTML form."""
+        if not self._renderedValueSet():
+            if self.hasInput():
+
+                # It's insane to use getInputValue this way. It can
+                # cause _error to get set spuriously.  We'll work
+                # around this by saving and restoring _error if
+                # necessary.
+                error = self._error
+                try:
+                    try:
+                        value = self.getInputValue()
+                    except InputErrors:
+                        tokens = self.request.form.get(self.name, 
self._missing)
+                        if isinstance(tokens, list):
+                            return 
[self.vocabulary.getTermByToken(token).value for token in tokens]
+                        elif not tokens:
+                            return []
+                        else:
+                            return 
[self.vocabulary.getTermByToken(tokens).value]
+                           
+                finally:
+                    self._error = error
+            else:
+                value = self._getDefault()
+        else:
+            value = self._data
+        return self._toFormValue(value)
+
     def renderItems(self, value):
         if value == self.context.missing_value:
             values = []
@@ -519,7 +549,6 @@
                               extra=self.extra))
         return '\n'.join(items)
 
-
 class MultiSelectWidget(ItemsMultiEditWidgetBase):
     """Provide a selection list for the list to be selected."""
 
@@ -539,27 +568,32 @@
 
     template = ViewPageTemplateFile('orderedSelectionList.pt')
 
+    def _available_values(self):
+        """Return a list of available values."""
+        # Not all content objects must necessarily support the attributes
+        values = getattr(self.context.context, self.context.__name__,
+                         self.context.missing_value)
+
+        # Convert the missing value into a list
+        if values == self.context.missing_value:
+            values = []
+
+        return values
+
     def choices(self):
         """Return a set of tuples (text, value) that are available."""
-        # Not all content objects must necessarily support the attributes
-        if hasattr(self.context.context, self.context.__name__):
-            available_values = self.context.get(self.context.context)
-        else:
-            available_values = []
         return [{'text': self.textForValue(term), 'value': term.token}
                 for term in self.vocabulary
-                if term.value not in available_values]
+                if not self.context.unique or term.value not in 
self._available_values()]
 
     def selected(self):
         """Return a list of tuples (text, value) that are selected."""
         # Get form values
         values = self._getFormValue()
-        # Not all content objects must necessarily support the attributes
-        if hasattr(self.context.context, self.context.__name__):
-            # merge in values from content
-            for value in self.context.get(self.context.context):
-                if value not in values:
-                    values.append(value)
+        # merge in values from content
+        for value in self._available_values():
+            if value not in values:
+                values.append(value)
 
         terms = [self.vocabulary.getTerm(value)
                  for value in values]
@@ -567,9 +601,21 @@
                 for term in terms]
 
     def __call__(self):
+        self._update()
         return self.template()
 
+    def _update(self):
+        """Set various attributes for the template."""
+        self.unique = self.context.unique
+        self.min_length = self.context.min_length
+        self.max_length = self.context.max_length
+        num_items = len(self._available_values())
+        self.need_add = (not self.context.max_length
+                         or num_items < self.context.max_length)
+        self.need_delete = num_items and num_items > 
self.context.min_length
+        self.need_up_and_down = num_items > 1
 
+
 class MultiCheckBoxWidget(ItemsMultiEditWidgetBase):
     """Provide a list of checkboxes that provide the choice for the 
list."""
 
Index: browser/orderedSelectionList.pt
===================================================================
--- browser/orderedSelectionList.pt    (revision 71100)
+++ browser/orderedSelectionList.pt    (working copy)
@@ -1,50 +1,98 @@
 <script type="text/javascript">
 
-function moveItems(from, to)
-  {
+function moveItems(from, to, del_from, add_to){
   // shortcuts for selection fields
   var src = document.getElementById(from);
   var tgt = document.getElementById(to);
 
   if (src.selectedIndex == -1) selectionError();
-  else
-    {
+  else {
     // iterate over all selected items
     // --> attribute "selectedIndex" doesn't support multiple selection.
     // Anyway, it works here, as a moved item isn't selected anymore,
-    // thus "selectedIndex" indicating the "next" selected item :)
-    while (src.selectedIndex > -1)
-      if (src.options[src.selectedIndex].selected)
-        {
-        // create a new virtal object with values of item to copy
-        temp = new Option(src.options[src.selectedIndex].text,
-                      src.options[src.selectedIndex].value);
-        // append virtual object to targe
-        tgt.options[tgt.length] = temp;
-        // want to select newly created item
-        temp.selected = true;
-        // delete moved item in source
-        src.options[src.selectedIndex] = null;
+    // thus "selectedIndex" indicati ng the "next" selected item :)
+    while (src.selectedIndex > -1) {
+      if (src.options[src.selectedIndex].selected) {
+        if (add_to == true) {
+          // create a new virtal object with values of item to copy
+          temp = new Option(src.options[src.selectedIndex].text,
+                            src.options[src.selectedIndex].value);
+          // append virtual object to targe
+          tgt.options[tgt.length] = temp;
+          // want to select newly created item
+          temp.selected = true;
+        } else {
+          // don't create a new item because it already exists
+          // only highlight the item on the target side
+          var src_value = src.options[src.selectedIndex].value
+          for (var i=0; i < tgt.length; i++) {
+            if (tgt.options[i].value == src_value) {
+              tgt.options[i].selected = true;
+              break;
+            }
+          }
+        }
+        if (del_from == true) {
+          // delete moved item in source
+          src.options[src.selectedIndex] = null;
+        } else {
+          // do not move the item because it can be multi-selected
+          src.options[src.selectedIndex].selected = false;
+        }
       }
     }
   }
+}
 
+function hideButton(id) {
+  var button = document.getElementById(id);
+  button.disabled = true;
+}
+
+function showButton(id) {
+  var button = document.getElementById(id);
+  button.disabled = false;
+}
+
+function handleMinMax(name, min_length, max_length){
+  var length = document.getElementById(name+".to").length;
+  if (length > min_length) {
+    showButton(name+".to2fromButton")
+  } else {
+    hideButton(name+".to2fromButton")
+  }
+  if (length >= max_length) {
+    hideButton(name+".from2toButton")
+  } else {
+    showButton(name+".from2toButton")
+  }
+}
+
 // move item from "from" selection to "to" selection
-function from2to(name)
-  {
-  moveItems(name+".from", name+".to");
+function from2to(name, unique, min_length, max_length){
+  if (unique == 'True') {
+    moveItems(name+".from", name+".to", true, true);
+  }
+  else {
+    moveItems(name+".from", name+".to", false, true);
+  }
+  handleMinMax(name, min_length, max_length);
   copyDataForSubmit(name);
-  }
+}
 
 // move item from "to" selection back to "from" selection
-function to2from(name)
-  {
-  moveItems(name+".to", name+".from");
+function to2from(name, unique, min_length, max_length){
+  if (unique == 'True') {
+    moveItems(name+".to", name+".from", true, true);
+  }
+  else {
+    moveItems(name+".to", name+".from", true, false);
+  }
+  handleMinMax(name, min_length, max_length);
   copyDataForSubmit(name);
-  }
+}
 
-function swapFields(a, b)
-  {
+function swapFields(a, b) {
   // swap text
   var temp = a.text;
   a.text = b.text;
@@ -57,7 +105,7 @@
   temp = a.selected;
   a.selected = b.selected;
   b.selected = temp;
-  }
+}
 
 // move selected item in "to" selection one up
 function moveUp(name)
@@ -149,15 +197,25 @@
       </select>
     </td>
     <td>
-      <button name="from2toButton" type="button" value=" -&gt;"
+      <button name="from2toButton" id="from2toButton" type="button" 
value="&rarr;"
           onclick="javascript:from2to()"
-          tal:attributes="onClick 
string:javascript:from2to('${view/name}')"
-          >&nbsp;-&gt;</button>
+          tal:attributes="onClick 
string:javascript:from2to('${view/name}', '${view/unique}', 
'${view/min_length}', '${view/max_length}');
+                          id string:${view/name}.from2toButton"
+          >&rarr;</button>
       <br />
-      <button name="to2fromButton" type="button" value="&lt;- "
+      <script type="text/javascript" tal:condition="not: view/need_add" 
tal:content="string:
+          hideButton('${view/name}.from2toButton');">
+          // hide from2toButton
+      </script>
+      <button name="to2fromButton" id="to2fromButton" type="button" 
value="&larr;"
           onclick="javascript:to2from()"
-          tal:attributes="onClick 
string:javascript:to2from('${view/name}')"
-          >&lt;-&nbsp;</button>
+          tal:attributes="onClick 
string:javascript:to2from('${view/name}', '${view/unique}', 
'${view/min_length}', '${view/max_length}');
+                          id string:${view/name}.to2fromButton"
+          >&larr;</button>
+      <script type="text/javascript" tal:condition="not: 
view/need_delete" tal:content="string:
+          hideButton('${view/name}.to2fromButton');">
+          // hide to2fromButton
+      </script>
     </td>
     <td>
       <select id="to" name="to" size="5" multiple=""
@@ -180,16 +238,16 @@
     </td>
     <td>
       <button
-          name="upButton" type="button" value="^"
+          name="upButton" id="upButton" type="button" value="&uarr;"
           onclick="javascript:moveUp()"
           tal:attributes="onClick string:javascript:moveUp('${view/name}')"
-          >^</button>
+          >&uarr;</button>
       <br />
       <button
-          name="downButton" type="button" value="v"
+          name="downButton" id="downButton" type="button" value="&darr;"
           onclick="javascript:moveDown()"
           tal:attributes="onClick 
string:javascript:moveDown('${view/name}')"
-          >v</button>
+          >&darr;</button>
     </td>
   </tr>
 </table>
Index: browser/sequencewidget.py
===================================================================
--- browser/sequencewidget.py    (revision 71100)
+++ browser/sequencewidget.py    (working copy)
@@ -47,6 +47,14 @@
     def __init__(self, context, field, request, subwidget=None):
         super(SequenceWidget, self).__init__(context, request)
         self.subwidget = subwidget
+        # All AbstractCollection fields have a `_type` attribute specifying
+        # the type of collection. Use it to generate the correct type if
+        # available.  TODO: this breaks encapsulation.
+        if hasattr(self.context, '_type'):
+            _type = self.context._type
+            if isinstance(_type, tuple):
+                _type = _type[0]
+            self._type = _type
 
         # The subwidgets are cached in this dict if preserve_widgets is 
True.
         self._widgets = {}
@@ -125,7 +133,10 @@
     def _getRenderedValue(self):
         """Returns a sequence from the request or _data"""
         if self._renderedValueSet():
-            sequence = list(self._data)
+            if self._data is not self.context.missing_value:
+                sequence = list(self._data)
+            else:
+                sequence = []
         elif self.hasInput():
             sequence = self._generateSequence()
         elif self.context.default is not None:
@@ -157,6 +168,7 @@
         """
         if self.hasInput():
             self.preserve_widgets = True
+            # convert the sequence to the field-specific implementation
             sequence = self._type(self._generateSequence())
             if sequence != self.context.missing_value:
                 # catch and set field errors to ``_error`` attribute
Index: browser/tests/test_itemswidget.py
===================================================================
--- browser/tests/test_itemswidget.py    (revision 71100)
+++ browser/tests/test_itemswidget.py    (working copy)
@@ -437,7 +437,7 @@
    
     def test_hidden(self):
         widget = self._makeWidget(
-            form={'field.numbers': ['two','three']})
+            form={'field.numbers': ['token2','token3']})
         widget.setPrefix('field.')
         widget.context.required = False
         self.verifyResult(
Index: utility.py
===================================================================
--- utility.py    (revision 71100)
+++ utility.py    (working copy)
@@ -221,7 +221,7 @@
                 # warning.
                 viewType = IInputWidget
         setUpWidget(view, name, field, viewType, value, prefix,
-                    ignoreStickyValues, context)
+                    ignoreStickyValues, source)
         res_names.append(name)
     return res_names
 





More information about the Zope3-dev mailing list