Wrap icon and text in <a> tag

[markconroy]

Closes #787

What does this change?

Moves the icon in the link paragraph into the a tag so both the text and the icon are clickable.

[msayoung]

This works fine for the default styling, but the "button" styling still has an area which looks clickable (and changes on hover) but isn't clickable.

[markconroy]

@msayoung Can you test this again please? It looks to me that the template only renders if there is a URL (without a URL, it's basically just a text field).

I've added:

1. Link with a URL - text + icon is clickable
2. Button with a URL - text + icon is clickable
3. Link without a URL - nothing displays on page from template
4. Button without a URL - nothing displays on page from template

Here's a screenshot of the edit page (you can see the two extra items) followed by a screenshot of the rendered page (only the items with URLs are showing). So, I think this is working correctly in terms of "icon + text should be clickable".

[msayoung]

@markconroy - I think we are looking at different things.

The issue is that with the button style, there is a hover effect on the non-interactive element.

<div class="link link--button-style paragraph paragraph--type--localgov-link paragraph--view-mode--default">
      <div class="link-wrapper">
      <div class="link-content">
        <a href="https://agile.coop">
        <div aria-hidden="true" class="lgd-icon link-icon">
          <svg xmlns="http://www.w3.org/2000/svg" focusable="false" viewBox="0 0 320 512">...
        </svg>
        </div> This is a link
        </a>
      </div>
    </div>
</div>

The hover effect is on the .link-wrapper , which is much larger than the , making it look like the whole thing is clickable.

Does that make sense?

[markconroy]

Thanks @msayoung I see what you mean now.

I've added some CSS so the whole component is clickable.

[ctorgalson]

No objections, just one small suggestion. I can see that this should work as intended.

I do wonder about the need for .link-content > a::after though--it would be simple enough to shift the styles from .link-wrapper into .link-content > a so that the link itself occupied 100% of .link-wrapper.

Is it just a cautious approach to lessen the risk of stepping on the toes of child-theme implementations?

[ctorgalson]

If the only content of .link-wrapper is .link-content, I think line 3 is superfluous, and line 4 belongs after line 8.

[markconroy]

@ctorgalson

I do wonder about the need for .link-content > a::after though--it would be simple enough to shift the styles from .link-wrapper into .link-content > a so that the link itself occupied 100% of .link-wrapper.

I think I'd like to leave this as is "just in case" someone wants the link to be 100% of the .link-content (which in our default set up is the same as 100% of .link-wrapper) but might also want some padding/different colour, etc on .link-wrapper.

[markconroy]

@msayoung I think i have your issues resolved, but it still says "Changes requested" by you. Can you hit the approve button if you don't have any more changes requested please.