-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
fixes #64: SingleCheckBoxFieldWidget does not render value in view mode #65
Conversation
828b3a6
to
4218345
Compare
plone/app/z3cform/converters.py
Outdated
# this way it works with one checkbox | ||
if value and value[0] == 'selected': | ||
return True | ||
return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why isn't the existing BoolSingleCheckboxDataConverter in z3c.form sufficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because there is no value for no
for display mode. This make it explicit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whether we set up the display value here or in the widget, we should probably use messageids rather than hardcoded strings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the vocabulary used from z3c.form uses msgstr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right, I even saw that the other day, and then forgot.
layer="plone.app.z3cform.interfaces.IPloneFormLayer" | ||
class=".templates.RenderSingleCheckboxWidget" | ||
permission="zope.Public" | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point of this view was to make sure that the label is rendered to the right of the checkbox in input mode and the description underneath. I see you made the label go in the right place, but I think with your change the description will now be shown above the checkbox.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
youre right, know, that we have an own widget for this, I can provide an own display template for this case in order to show the description right.
@agitator may you provide me with the markup needed for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
By the way, it's probably worth checking that this widget works correctly in hidden mode too. I've had trouble with that with single checkboxes in the past. |
probably the hidden mode is a problem too. |
@jensens this is still in progress or can it - after a rebase - be merged? |
@thet it is still WIP |
38818fb
to
7df904f
Compare
replaced existing hack by a real widget.