Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(z3cformdoc): One can't change widget factories in update() any more #247

Merged
merged 1 commit into from
Jan 24, 2014
Merged

Conversation

do3cc
Copy link
Member

@do3cc do3cc commented Jan 24, 2014

When using autoform, the widgets get initialized before the update
method is run. Therefore one must override the widgetFactory in updateWidgets.

Dunno if this was intended, but thats the way it works now.

When using autoform, the widgets get initialized before the update
method is run. Therefor one must override the widgetFactory in updateWidgets.
hvelarde added a commit that referenced this pull request Jan 24, 2014
fix(z3cformdoc): One can't change widget factories in update() any more
@hvelarde hvelarde merged commit 72af3c4 into collective:master Jan 24, 2014
@@ -676,7 +674,7 @@ Example from `collective.z3cform.datagridfield_demos <https://github.com/collect

grok.name('demo-collective.z3cform.datagrid-block-edit')

def update(self):
def updateWidgets(self):
# Set a custom widget for a field for this form instance only
self.fields['address'].widgetFactory = BlockDataGridFieldFactory
super(EditForm9, self).update()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're changing this to updateWidgets, you need to call updateWidgets on the super too, or you'll get into a loop.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed (I guess) in 982af27

@do3cc
Copy link
Member Author

do3cc commented Jan 24, 2014

@davisagli you said that calling update() in updateWidgets would make the view run in circles. That made me wonder and look more into the issue.
a z3c.form view really calls updateWidgets() from within update. In such a case I could still add another widgetfactory in an update method. But plone.autoform changed the order in which the methods get called. There updateWidgets() is called before update().

https://github.com/plone/plone.autoform/blob/master/plone/autoform/view.py#L45
https://github.com/zopefoundation/z3c.form/blob/master/src/z3c/form/form.py#L148
For me this sounds like a bug in plone.autoform. Do you agree or do I miss something?

@davisagli
Copy link
Member

Here's the order of operations when update gets called on most autoforms:

  1. self.update, which is plone.z3cform.extensible.ExtensibleForm:update unless overridden
  2. self.updateFields, which is plone.autoform.form.AutoExtensibleForm:updateFields unless overridden
  3. self.updateFieldsFromSchemata, which is plone.autoform.base.AutoFields:updateFieldsFromSchemata unless overridden (here's where the fields are created, and default widgets set)
  4. super(AutoExtensibleForm, self).updateFields, which is usually plone.z3cform.extensible.ExtensibleForm:updateFields (this gives form extenders a chance to run; see @bosim's recent blog post)
  5. super(ExtensibleForm, self).update, which is usually z3c.form.group.GroupForm:update
  6. self.updateWidgets, which is z3c.form.group.GroupForm:updateWidgets unless overridden

You pointed at plone.autoform.view.WidgetsView, which is used for views (not forms) that make use of z3c.form widgets in display mode, and it works somewhat differently. It does seem wrong that it calls updateWidgets before calling update, but I guess Martin structured it this way so that it would with views that are doing their own (non-widget-related) initialization in update. Overriding updateWidgets like in this PR should still work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants