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 select2 widgets rendering empty values in display mode when called from an easyform action #6

Open
wants to merge 1 commit into
base: mtneers
Choose a base branch
from

Conversation

chiruzzimarco
Copy link
Collaborator

Related to Mountaineers issue #3991.

I think the root cause for this issue relies on plone.app.z3cform.
When the form is submitted selected values for select2 widgets are stored in the request form as strings, but the Select2Widget extract method is not doing any kind of validation on the request input to ensure this is converted to a list like z3c.form is doing on the SequenceWidget.

Thus when the DisplayValue method is called it expects to iterate on a list, but it will instead iterate on a string and raise LookupErrors which are ignored.

This "hacky" solution is the best thing i could find without the need to edit directly z3c.form or plone.app.z3cform code.

Copy link

@alecpm alecpm left a comment

Choose a reason for hiding this comment

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

I don't like that this fix changes the value stored in the request during submit processing, especially since it's converting it to a value that isn't easily serializable in a widget if there's e.g. a validation exception on another field. It would be better to fix the specific issue which is the display mode rendering of the widget in the specific DummyFormView context. Perhaps a custom widgetTemplate directive that specifies the view class? I also. don't love that this fix lives in collective.easyform. I think we should try to register a custom display widget or widgetTemplate for the specific view class somewhere in the mtneers_shared package instead.

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.

2 participants