Skip to content

Hotfix/input text valign#1105

Closed
Chiu-Kevin-bah wants to merge 3 commits into
uswds:18f-pages-stagingfrom
Chiu-Kevin-bah:hotfix/input-text-valign
Closed

Hotfix/input text valign#1105
Chiu-Kevin-bah wants to merge 3 commits into
uswds:18f-pages-stagingfrom
Chiu-Kevin-bah:hotfix/input-text-valign

Conversation

@Chiu-Kevin-bah

Copy link
Copy Markdown
  • There was a line-height like issue with the radio and checkbox rendering where the text was closer to the bottom than the vertical middle point. Only issue I see with this current setup is probably using odd numbers. It seems like though it's different between browsers as well as far as how off the center the text might be aligned. It's a vertical height difference with the font most likely,

I made a pretty safe fix with changing position to relative and setting a top value.

  • Side note, I had some trouble setting up the build system because I didn't realize node v12.0 was a dependency. Running 14.04 Ubuntu, I actually had 0.10.44 installed and took me some research to find out why I was getting an execSync error.

@maya

maya commented Apr 11, 2016

Copy link
Copy Markdown
Contributor

Thank you @Iceman52489 for your attention to detail! Getting vertically center aligned radio buttons and checkboxes is great. I'll reach out to a few people about this method. In general we try to avoid "magic numbers" and want something that will work if someone decides to change the font styles to a different pairing, so it's more reusable.

@Chiu-Kevin-bah

Chiu-Kevin-bah commented Apr 18, 2016

Copy link
Copy Markdown
Author

Hey @maya, took a dive into why I went with that magic number and seeing if I can get a more automated number. Can you take a look the following screenshots along with states (These were taken from the live site so unless mentioned, it wasn't changed through console):

line-height: 1.5;

line-height: 0.8;
vertical-align: 0.2em;

For example, if you take out the 2 styles above (obviously defaulting to defaulting position we end up with this:

Then I decided to look into how that outer ring was achieved and if there’s was anyway we can get the “correct” height calculation so positioning was accurate. Couldn’t really find anything to why the position is thrown off because padding or border wasn’t used at all.

THEN, I found out that you guys used box-shadow: 0 0 0 2px #ffffff, 0 0 0 4px #0071bc;. I don’t think it’s normal for box-shadow directly affect element position (maybe just for pseudo elements, I didn’t test it). But if I set box-shadow: none !important; and line-height: 1; so that we can sync up the line height with the label, here’s what happens:

So I think the 1px magic value isn’t quite so magic (although it was trial and error initially), it turns out, I got lucky.

Wanted to point out that it looks like it might not be perfect still but that’s probably because you’re trying to vertically center an element whose height is even where the parent height is an odd number by browser calculation.

@maya

maya commented Apr 18, 2016

Copy link
Copy Markdown
Contributor

Hello @Iceman52489!

I was looking at your PR and it was still a little off to me, nothing I tried was working until I managed to reset everything by removing line-height AND vertical align. After that, I added line-height based off of the checkbox and radio button height and width, and it aligned perfectly. Best of all, no magic numbers! 🎉

You were on the right track by removing line-height and vertical align, but adding in line-height with the value of the size of the input is what got it to work consistently.

Also, box-shadow was not effecting positioning for me.

I opened a PR with the result here: #1127

@msecret

msecret commented May 10, 2016

Copy link
Copy Markdown
Contributor

I'm going to close this issue as it looks like it was fixed in #1127

@msecret msecret closed this May 10, 2016
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.

3 participants