Skip to content
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

Removed Right Align and Fixed With of Inputs. #228

Merged
merged 4 commits into from
Jun 21, 2021

Conversation

michaelhiiva
Copy link
Contributor

@michaelhiiva michaelhiiva commented Jun 19, 2021

Summary

The Sign In Page currently aligns each label to the right. Cleans up Sign In Display.

This change removes that and assures that the password and
text input boxes are the same width.

Screenshots

Current

Screenshot from 2021-06-18 17-22-45

Updated

Screenshot from 2021-06-18 17-20-23

The Sign In Page currently aligns each label to the right.
This change removes that and assures that the password and
text input boxes are the same width.
@michaelhiiva michaelhiiva self-assigned this Jun 19, 2021
@michaelhiiva michaelhiiva requested a review from neagle June 19, 2021 00:22
@neagle
Copy link
Collaborator

neagle commented Jun 19, 2021

Thanks for the PR, @EarthSchlange! I'm glad you were able to get things running locally. :)

For the text alignment, I definitely think the login could use some custom design treatment at some point, but I actually don't like having the labels aligned left in this case. It leaves the labels drifting off in space, separated by a large and uneven expanse from the text fields to which they're related.

I can see why the right-aligned version might look broken or unappealing, since the alignment of the login fields seems unrelated to the "Sign In" header, but I'd rather solve that design problem a different way.

Adding the [type="password"] to the CSS to make its width the same is a good fix, though. I'd be happy to merge that in.

@michaelhiiva
Copy link
Contributor Author

michaelhiiva commented Jun 19, 2021 via email

@michaelhiiva
Copy link
Contributor Author

This commit removes the labels and uses placeholder instead. @neagle would this approach work for you?

Screenshot from 2021-06-19 13-59-56

@neagle
Copy link
Collaborator

neagle commented Jun 20, 2021

Your approach definitely looks good from an aesthetic perspective, but there are important problems with using the placeholder attribute that way.

https://www.smashingmagazine.com/2018/06/placeholder-attribute/

A better approach that might fix your dislike of the right alignment is the one recommended toward the end of that article, where the label is placed above the relevant input:

2021-06-20 at 11 17 AM

That said, you should note that the styles you're editing right now affect not just the sign in, but form fields all over the site.

2021-06-20 at 11 19 AM

That's one reason I've been gradually changing all the forms to use standard .field-key and .field-value markup, so that they can first be made consistent, and then we can potentially devote some more time to form design. This is also in preparation for making the forms work better in narrow, mobile views.

@michaelhiiva
Copy link
Contributor Author

@neagle Thank you for the smashing magazine article. The article has two important points accessibility and language. According to this smashing magazine article the placeholder attribute is "skipped over by browsers." This could mean some of our users will not get the full experience of our website.

https://www.smashingmagazine.com/2018/06/placeholder-attribute/

The solution of placing a label above is reasonable with the addition of subtext to describe the input.

@michaelhiiva
Copy link
Contributor Author

Removes the flex display. I don't think we need a description below the email at this point.

Screenshot from 2021-06-20 15-54-17

@neagle
Copy link
Collaborator

neagle commented Jun 21, 2021

Thanks, @EarthSchlange!

@michaelhiiva michaelhiiva deleted the fix_sign_in_textbox_password branch June 28, 2021 21:47
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