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




_______________________________________________
Zope3-dev mailing list
Zope3-dev@zope.org
Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com

Reply via email to