Remove horizontal layout from Featured Menu Content to fix a11y img alt issue

[anselmbradford]

Images inside links should have alt text that describes the link. This was failing our wcag tests.

Additions

• Adds alt text to mega menu image links.

Testing

1. gulp test:a11y should only show 1 error, not 5.

Screenshots

[jimmynotjim]

I thought if the alt text duplicates the content the best practice was not to use an alt text rather than have the screen reader repeat the content?

[anselmbradford]

Well the way we have it set up the image is alone in a link, even though there's the same link with text nearby, so I'm not sure, would the screenreader jump to the link? WCAG fails it on https://achecker.ca/checker/suggestion.php?id=7

[Scotchester]

The link should wrap both the image and heading, if possible.

[jimmynotjim]

Ahhhh, yep, looks like we could update the markup and less to simplify the component a bunch. I think the __horizontal modifier is no longer necessary so we can eliminate all the display: table stuff.

[anselmbradford]

@schaferjh https://[GHE]/flapjack/Modules-V1/wiki/Featured-Menu-Content has a horizontal layout for featured menu content. Is that still something we'll have at some point?

[schaferjh]

@anselmbradford given how much our navigation items have expanded, I doubt it.

[anselmbradford]

Removed horizontal layout. Ready for review again.

[jimmynotjim]

Now that the link wraps the image and the text, we really don't want alt text right?

[anselmbradford]

Err right. Removed the alt values. wcag is failing on the images now unfortunately. Looks like it doesn't recognize the text inside the link, but it is correct according to https://achecker.ca/checker/suggestion.php?id=7

[Scotchester]

Looks like the images need display: block;:

[anselmbradford]

@Scotchester Fixed the image and text.

[Scotchester]

Left one more semantics/accessibility question, but it doesn't have to be a blocker for this PR.

It'd be nice to do some accessibility-focused user testing of our menu.

[Scotchester]

Sorry, one more thing. Should this text be in a heading element, not a paragraph? It's a little confusing to me because it's within the navigation. I realize I'm not sure what the right way to handle headings in complex navigation is. Presumably we'd want a screen reader to read things like the "Money topics" and "Guides" headings as one tabs through the navigation, but I'm not sure if they will be, or if it just skips to links. Something to test, I suppose, but we're venturing into out-of-scope territory.

If you think we should go ahead and make it a heading here, though, I don't think a heading can contain an image, but you could probably do:

aside
    a
        img
        h4
    p

If not, though, I might suggest restructuring it that way, anyway, so you can eliminate the wrapping p.h4 and just use a div class="h4" around the {{ value.link.text }}.

[anselmbradford]

Could I use <h4> instead of <div class="h4">? Either looks like it would eliminate the _text modifier.

[Scotchester]

That was my initial suggestion :) I only started waffling because I realized I was unsure about best practices (regarding accessibility) for using headings within complex navigation.

[anselmbradford]

Ahh right "heading element" got it. I did read it! I swear! I'll set it to the div for now as that's the safer of the two.

[Scotchester]

Approved again :) Merge away!