[Footer]: Fix footer social icon hit area#1248
Conversation
## Description Fixes #346 ## Additional information * This PR uses `display: inline-block` to allow setting height and width to an inline element. * Uses 44 pixels for both height and width per the Apple iOS HIG * Concerns with `text-align: right` - could result in thumb hitting to the right of the hit area - actually hitting the icon to the right of the actual target. `text-align: center` used instead. * Using `text-align: center` causes the right alignment of the icons to be off when compared “Agency Contact Center” text. `position: relative; left: 13px` used in order to compensate. Slight concern that this could cause a horizontal scroll bar to appear on mobile - though, given the overall alignment of the `social-links` container - concern may be moot.
|
PR solution:
PR solution:
|
|
@colinpmacarthur - Just wanted to let you know and get your thoughts. |
|
@joshbruce thanks for opening this PR! Just wanted to bring up this currently closed issue #706 with new footer designs. In this design, we're using larger icons to expand the hit area. But your solution of just expanding the hit area and keeping the icons the same size also works. Mostly, I'd like to know which approach our designers prefer - @ericadeahl @bradnunnally |
|
@maya - Thanks! I like that design direction a great deal. I don't see a specified height and width in the comp for the icons or the resulting hit area. Having said that, this setup could be used as a foundation for #706. If either dimension is less than 44px; or, we could maintain a square hit area - matching width (seems the smaller of the two) to height. |
|
@maya Sounds good to me to keep the larger icons and also make sure the hit area is at least 44px in width and height. It looks to me like the design files already have the larger icons incorporated, but the Standards site still uses the smaller ones. |
|
@ericadeahl thanks for checking! I left a note in the issue #346 to use the larger icons and will be closing this PR. |
|
@joshbruce sorry if I closed this too early, let me know if you'd like to implement the larger icons on this PR and I'd be happy to reopen. |
|
@maya - I should be able to do that. Because we are using SVG, I should be able to just change the dimensions - and update the hit area to accommodate. From what I can tell, the "larger" icons are not modified structurally from those already used, is that a correct assumption? If you reopen the PR I should be able to make the minor modifications to get us there. |
|
@joshbruce sure! It's SVG + PNG fallback if you look at the markup. Also, CSS properties must be alphabetized and use REM/EM values (per https://pages.18f.gov/frontend/css-coding-styleguide/) |
|
@maya - Thanks! Sorry for missing the PNG fallback - skimming the code quickly. :) Curious to know the rationale regarding the alphabetical ordering as opposed to groupings similar to: http://codeguide.co/#css-declaration-order - I'm not always good at doing this, but it is how I typically look at things. I should be able to do the update next week. I might end up submitting an alternative PR to accomplish the same thing, just a little more flexibility, imho. |
|
@ericadeahl - I cloned https://github.com/18F/web-design-standards-assets - I'm not sure if I'm seeing the larger icon set in there - can you give me the target dimensions for the icons (sorry, I don't see the dimensions in the comp from #706)? The Facebook icon width:height in the AI file I have is ~ 13:27 pixels. |
Good question! We find it's easier to find where properties are when it's alphabetical and easier for people to meet the standards and to review and lint - especially on such a large code base with external contributors. Our coding styleguide says we can use either on a project as long as it's consistent, but we've gone with alphabetical for the previous reasons. |
|
@joshbruce Sorry for the confusion; we haven't implemented the comps from #706. The larger icon size I was referring to is the one from the current Illustrator file (see screenshot.) The icons there appear to be larger than the ones on standards.usa.gov. The dimensions from the design file are (approximately):
|
|
We've also been using 2x pixel sizes for retina screens. |
- changed icon base sizes - made 2x for high-density displays - converted to rem - hit area larger than 44px
|
|
|
@joshbruce you should be able to do that with the svg and img size and do not need to change the svg files. |
|
See also: #1262 |
|
@joshbruce Looks like you'll need to rebase this branch onto the latest work on |
|
Awesome, thanks @maya |
Description
Fixes #346
Additional information
display: inline-blockto allow setting height andwidth to an inline element.
text-align: right- could result in thumb hitting tothe right of the hit area - actually hitting the icon to the right of
the actual target.
text-align: centerused instead.text-align: centercauses the right alignment of the icons tobe off when compared “Agency Contact Center” text.
position: relative; left: 13pxused in order to compensate. Slight concern that this couldcause a horizontal scroll bar to appear on mobile - though, given the
overall alignment of the
social-linkscontainer - concern may be moot.