Opened 11 years ago

Closed 11 years ago

Last modified 11 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:


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.

Change History (11)

comment:1 by Jef van Schendel, 11 years ago

Component: programminggraphic design/interface

comment:2 by Jakob Kramer, 11 years ago

The problem is that the string is specified in “description” instead of “label” in edit/ 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. There is an example in the WTForms documentation.

Version 1, edited 11 years ago by Jakob Kramer (previous) (next) (diff)

comment:3 by Emily O'Leary, 11 years ago

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 by Emily O'Leary, 11 years ago

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:

comment:5 by spaetz, 11 years ago

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 by Emily O'Leary, 11 years ago

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

comment:7 by Jef van Schendel, 11 years ago

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 by spaetz, 11 years ago

Cc: sebastian@… added
Milestone: 0.3.3

comment:9 by spaetz, 11 years ago

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:// 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 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:

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

comment:10 by Christopher Allan Webber, 11 years ago

Resolution: fixed
Status: acceptedclosed

Merged, pushed, closed. Thanks LotusEcho & Spaetz!

comment:11 by Christopher Allan Webber, 11 years ago

Note: See TracTickets for help on using tickets.