Different widths of input fields in edit profile

[michael]

When you edit your profile you can see different widths of input fields and textareas.

If you add "width: 100%" to the input fields all input elements have the same width.

[Jeremy Pope]

I do have this set in the sandy theme, should apply it to the default and airy themes as well.

[Vijeth]

Issue 996 : Fixes fields' width consistency

[Vijeth]

I'm new to MediaGlobin and this would be my first fix :)

I have attached a patch file which fixes the issue. I have currently removed the use of wtforms_util in edit_profile.html as I wanted to make width of the fields 100%. If there is a better, easier way, please let me know and I will make the required changes!

[ayleph]

Hi varadhya,

Thanks for looking into this issue and writing up a patch. I don't think this is the best way forward though. We use the wtforms template to reduce code in many other templates. Your proposed change is to remove the use of wtforms in the edit_profile template. This now leaves us with two issues.

1. Other templates that rely on wtforms still have mixed-length fields.

2. We now have duplicated code between edit_profile and wtforms.

I think it would be best if we could continue using the wtforms template to reduce duplicated code in other templates. This helps make the code more consistent from a programming standpoint. Maybe we could modify the wtforms template itself to set the correct widths so that all of the other templates that rely on wtforms become uniform visually as well.

[Vijeth]

modify render div in util/wtforms

[Vijeth]

Screenshot to verify the change

[Vijeth]

Hello ayleph,

I thought of doing exactly that, but I thought it would affect other pages which use wtforms in ways it shouldn't (even in the bug description, it hasn't specified that width should be 100% for other pages also).

But, anyways sorry for the delay, I have attached a patch file which tweaks the wtforms itself (and a screenshot which verifies that) :)

Thanks!

[ayleph]

Hey varadhya, thanks for making the changes. I started reviewing this but I ran into a problem. On the user registration page (mediagoblin.example.org/auth/register), there's some javascript code (show_password.js) that handles the "Show password" function. When I check the "Show password" box, the full-width password field shrinks down to a smaller size. When I uncheck the box, it grows back to 100%. I tried modifying the show_password.js file to specify 100% width for the duplicate password field, but that ends up just hiding the checkbox completely. Do you have any thoughts?

[ayleph]

Well, I found a way around that, but it's not perfect. I set the duplicate password field width to 100% in show_password.js. Then, instead of creating the "Show password" checkbox in javascript, I can re-create in in the wtform view. It actually looks more consistent that way, and we can apply translation strings to it.

diff --git a/mediagoblin/plugins/basic_auth/forms.py b/mediagoblin/plugins/basic_auth/forms.py
index 3d684e91..e973290e 100644
--- a/mediagoblin/plugins/basic_auth/forms.py
+++ b/mediagoblin/plugins/basic_auth/forms.py
@@ -28,6 +28,9 @@ class RegistrationForm(wtforms.Form):
         _('Password'),
         [wtforms.validators.InputRequired(),
          wtforms.validators.Length(min=5, max=1024)])
+    password_boolean = wtforms.BooleanField(
+        label='',
+        description='Show password')
     email = wtforms.StringField(
         _('Email address'),
         [wtforms.validators.InputRequired(),
diff --git a/mediagoblin/static/js/show_password.js b/mediagoblin/static/js/show_password.js
index b3fbc862..911cce2b 100644
--- a/mediagoblin/static/js/show_password.js
+++ b/mediagoblin/static/js/show_password.js
@@ -18,7 +18,7 @@
 
 $(document).ready(function(){
   //Create a duplicate password field. We could change the input type dynamically, but this angers the IE gods (not just IE6).
-  $("#password").after('<input type="text" value="" name="password_clear" id="password_clear" /><label><input type="checkbox" id="password_boolean" />Show password</label>');
+  $("#password").after('<input type="text" value="" name="password_clear" id="password_clear" style="width:100%" />');
   $('#password_clear').hide();
   $('#password_boolean').click(function(){
     if($('#password_boolean').prop("checked")) {

But there's a downside. With the current code, the "Show password" checkbox is only shown if the user allows javascript to run. With my change above, the "Show password" checkbox is shown all the time. But if the user doesn't allow javascript to run, then the checkbox doesn't actually do anything. So that might be confusing to the user.

I guess a simple work-around would be to change the text from "Show password" to "Show password (Javascript must be enabled)" or something like that.

Edit: As a bonus, if we move the label text out of show_password.js, that helps with #417.

[Vijeth]

Hi ayleph,

---

I tried modifying the show_password.js file to specify 100% width for the duplicate password field, but that ends up just hiding the checkbox completely

---

Why can't we add a <br> statement and shift the checkbox to the next line?
IMHO it's not a good idea to mess with JS disabled scenario and put a comment like that :/

$("#password").after('<input type="text" value="" name="password_clear" id="password_clear" style="width:100%" /><label><br><input type="checkbox" id="password_boolean" />Show password</label>');

I checked it out, it works! :)

[ayleph]

Why can't we add a <br> statement and shift the checkbox to the next line?

That was my first idea too, but I couldn't make it work. I think I was putting it outside the <label> tag though, so I guess that explains it. Thanks for the suggestion! I'll continue reviewing.

[ayleph]

I checked through most of the user-facing forms affected by this change and didn't see anything else that looked like it needed to be addressed. I didn't check all of the plugin templates affected by this change, as I don't have a good way to test the external authentication plugins. I've attached a slightly updated patch including the change to show_password described above. I'd go ahead and merge myself, but my computer reports that the Savannah git host machine ssh key has changed and I'd like to get confirmation that this is expected before I push.

[ayleph]

This has been merged in ​96f66a5.