Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#475 closed defect (fixed)

Checkbox for email notifications sits above its label

Reported by: Jef van Schendel Owned by: Emily O'Leary
Priority: major Milestone: 0.3.2
Component: graphic design/interface Keywords:
Cc: sebastian@… Parent Tickets:

Description

On the "Changing account settings" page, the checkbox for email comment notifications looks like this:

[✓]
Email me when others comment on my media

It should look like this:

[✓] Email me when others comment on my media

And clicking the label should check or uncheck the checkbox.

To do this, the HTML should be changed from the current:

<input type="checkbox">
<p>Email me when others comment on my media</p>

To this:

<label><input type="checkbox">Email me when others comment on my media</label>

But I have no idea how to do this, because it's generated by WTForms.

Subtickets

Change History (11)

comment:1 Changed 11 years ago by Jef van Schendel

Component: programminggraphic design/interface

comment:2 Changed 11 years ago by Jakob Kramer

The problem is that the string is specified in “description” instead of “label” in edit/forms.py. But when you specify it as label, the check box is behind the label. So you have to change templates/mediagoblin/utils/wtforms.html to render check boxes in another way (“field” first, then label). There is an example in the WTForms documentation.

Last edited 11 years ago by Jakob Kramer (previous) (diff)

comment:3 Changed 10 years ago by Emily O'Leary

Owner: set to Emily O'Leary
Status: newaccepted

I'm working on a change to implement this without possibly hurting existing uses of checkboxes.

comment:4 Changed 10 years ago by Emily O'Leary

I made it custom render the Email commenting checkbox and use the standard wtforms.html rendering for the rest of the items. It should be ready for review.

You can find the relevant changes here:

https://gitorious.org/~lotusecho/mediagoblin/lotusechos-mediagoblin/commits/trac_475_email_notification_checkbox

comment:5 Changed 10 years ago by spaetz

Rather than hacking in a new WTforms renderer (which is a cool solution, I haven't looked at the code yet), one could also manually create the page using things like: {{ form.password.label }} {{ form.password }} rather than simply having the form being auto-generated (which is quite convenient on the other hand).

comment:6 Changed 10 years ago by Emily O'Leary

That is exactly what I ended up doing. Custom rendering in that equated to manual field placement for that page.

comment:7 Changed 10 years ago by Jef van Schendel

I just tried LotusEcho's patch, it works well on my end.

See, you guys are much smarter than me. Thanks for working on this. :)

comment:8 Changed 10 years ago by spaetz

Cc: sebastian@… added
Milestone: 0.3.3

comment:9 Changed 10 years ago by spaetz

Hi LotusEcho.

I had already typed a long reply but somehow it got lost, so here we go again. I had a look at your patch and saw that you do exactly what I had proposed. Ooops, mea culpa, I should have looked first :-).

So +1 on your patch. actually I believe I improved your patch and believe we should check in the improved version.

You can find it at my git repo git://gitorious.org/~spaetz/mediagoblin/spaetz-mediagoblin.git in branch trac_475_email_notification_checkbox. The commit is still credited to LotusEcho, of course. My improvements:
Original patch modified by Sebastian Spaeth to 1) not translate the
checkbox label in the template, it is translated in forms.py already.
2) Simplify the HTML, manually constructing the <label> tag is not
necessary, WTforms does it automatically.

Unless I push an updated version, the link to the patch is: https://gitorious.org/~spaetz/mediagoblin/spaetz-mediagoblin/commit/dba5ef66563a5de4a8f0970f3d6fd6556d6d6f19

Please check my patch and see if you approve, I think it should go into 0.3.2 or 0.3.3.

comment:10 Changed 10 years ago by Christopher Allan Webber

Resolution: fixed
Status: acceptedclosed

Merged, pushed, closed. Thanks LotusEcho & Spaetz!

comment:11 Changed 10 years ago by Christopher Allan Webber

Milestone: 0.3.30.3.2
Note: See TracTickets for help on using tickets.