Skip to content

Fix select2 widgets rendering empty values in display mode when called from an easyform action#6

Open
chiruzzimarco wants to merge 1 commit intomtneersfrom
mtneers-issue/3991-fix-select-widget-display
Open

Fix select2 widgets rendering empty values in display mode when called from an easyform action#6
chiruzzimarco wants to merge 1 commit intomtneersfrom
mtneers-issue/3991-fix-select-widget-display

Conversation

@chiruzzimarco
Copy link
Copy Markdown
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.

@chiruzzimarco chiruzzimarco requested a review from alecpm March 27, 2024 17:01
Copy link
Copy Markdown

@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