More Cleanup of Widgets

More Cleanup of Widgets

Status

IsProposal

Authors

Garrett Smith, Marius Gedminas

Problems

1. IInputWidget?'s validate and hasValidInput methods aren't used (WHUI).
2. SimpleBrowserWidget?'s getInputValue potentially changes the internal state of the widget (by setting _error), which is bad - it's implicit/unexpected behavior that has already caused problems for some of us.
3. IWidget?'s error method reflects some old thinking on how peripheral elements (e.g. labels and tooltips) -- it should be made consistent with the last round of refactoring.

Proposed Solution

1. Delete hasValidInput from the interface and its sole implementation - no one's going to miss it.
2. Remove validate from IInputWidget?.
3. Add a new method _validate(value) to SimpleInputWidget?, which performs the default validation for the widget (i.e. passes the value through to the field's validate method). This method can be overridden by subclasses to change the default validation behavior for the widget.
4. Change getInputValue's validation logic to call _validate (see 3) instead of the field's.
5. Change IWidget? error to errors, which is a list of of errors generated during the last call to getInputValue. This attribute is used by widgets to display any errors associated with the widget.

   In most cases, if an error exists, the list will contain a single item.

6. Delete BrowserWidget?'s _error and its use.
7. Delete IWidgetInputErrorView? and its sole implementation.
8. Move the default HTML rendering into widget_macros.pt.
9. Modify applyWidgetChanges (utility.py) to set the widget error attr if widget.applyChanges raises an error:

           try:
               value = widget.applyWidgetChanges()
           except InputErrors, v:
               widget.errors.append(v)
               self.errors.append(v)
   

This addresses problem 2 above - it makes the widget state change explicit rather than a side effect of calling a seemingly unrelated method.

1. Change WidgetInputError? as follows:

            class IWidgetInputError(Interface):
              widget = Attribute("The widget the error is associated with.")
              message = Attribute("An error message to display to the user.")
              original = Attribute("An optional original error that caused the
                                    input error.") 
   
            class WidgetInputError(UserError):
              def __init__(self, widget, message, original=None):       
                 self.widget = widget self.message = message
                 self.original = original
   

The current definition/implementation is a bit muddled - it's also misused in almost all cases.

Risks

The obvious risk with this proposal is breaking existing widget and form implementations. We'll mitigate these risks by ensuring short-term backward compatibility with deprecation or confirming via the developer's list that specific changes will in fact not break existing implementations.