Rework clickable cards using CSS invisible overlay rather than JS event handler

[andymantell]

Description

Rework clickable cards using CSS invisible overlay rather than JS event handler

This avoids problems with using Ctrl-click, middle click, right click to open new tabs since the link just behaves as it would natively

Fixes #758
Fixes #749

Checklist

• Tested against our testing policy (Resolution, Browser & Accessibility)
• Follows our coding standards and style guide
• CHANGELOG entry

Issues observed during browser testing

• IE10 - image is not clickable in "card with an image" example. Ignoring, because IE10 is not supported and the experience is ok.
  Fixed. Don't know whether we want IE10 fixes lingering around, but it's relatively inoffensive, so might not hurt to have it for a while? Easily removed if we don't want it...

[andymantell]

@chrimesdev I've put both these selectors in here in order to be consistent with what came before. Is this still necessary? I assume it's backwards compatibility before the .nhsuk-card__link class was added

[chrimesdev]

@Fenwick17 as you did the original work with the clickable Card, are you OK to take a look at this? I'll also give it a review

[Fenwick17]

Spotted one issue that occurs with MacOS Hover Text.
The large-text view does not work for the content of the card due to the overlay, hovering the heading will display the large-text view, but not the body content.

Everything else appears to be working as you would expect.

[andymantell]

Thanks @Fenwick17 - do you mean these ones?

[andymantell]

Sorry, I see what you mean now. Using https://support.apple.com/en-gb/guide/mac-help/mchlb203bc78/mac

Let me have a think about this 🤔

[Fenwick17]

Added a gif of the issue, if it can't be resolved we will have to make a decision on whether it is acceptable as it is.
I would image any Windows software that does this would have a similar behaviour.

I believe NVDA with mouseover will also not read the body content due to the overlay, but I haven't tested that yet.

[andymantell]

yeah makes sense. I'm on windows so I'll see if there's anything similar here. And will have a think if there's anything sensible can be done

[andymantell]

@chrimesdev @Fenwick17 I'm not sure what to do about this. Surely anything that alters what HoverText "sees" would also affect what a screenreader sees. So if we start changing things for HoverText, we would negatively affect other tools.

I don't know if a perfect solution exists for this then. I think we're stuck choosing between two slightly imperfect solutions:

1. Go back to the JS way of doing this, and try and enhance it for more ways to open new tabs (Ctrl-Click, middle click etc). This will never catch all the ways though (right click / long press on mobile to open new tab from the context menu for example)

2. Go with the invisible overlay, and accept the hovertext deficiency.

Any thoughts? Have I missed any other options?

[Fenwick17]

I just performed some Browserstack testing and didn't spot any issues with the CSS only solution.
So the only issues we seem to have are not being able to copy text and no hover text on the paragraph.

For both of these I personally don't think they are show stoppers.
The text is meant to just be a little bit of extra context so there shouldn't really be a need to copy it.
The link itself ideally should be clear enough for a user to know what it means without needing the additional text and standard zoom behaviour will still work as a fallback.

The only thing that comes to mind at the moment is if we add visually hidden content to the link itself, so that it will show up with Hover Text.

<a class="nhsuk-card__link" href="./pages/examples.html">
  Examples<span class="nhsuk-u-visually-hidden">: Page layouts, text pages and components</span>
</a>

Then you would have to use aria-hidden on the <p>.
This is probably wildly overkill but I thought it was a cool workaround.

What do you think? Overall, I think we can proceed with what Andy has done.

[chrimesdev]

@Fenwick17 as you have done more work on this than I have, I am happy to go with what you think is best.

[Fenwick17]

Happy to move forward with this implementation

[chrimesdev]

@andymantell do you think this needs a little more information in the CHANGELOG around removing the JavaScript for the Card component? If anyone is importing and initialising the Card using ES6 and we have now removed it, would it be a breaking change rather than a fix?

[andymantell]

@chrimesdev Agreed. Done - moved it to 6.0.0 and added a note about removing JS

[chrimesdev]

Thanks Andy, as this is a breaking change we will hold off merging this PR and merge/release with #740

[DomBaker]

Reviewed and approved. ready for v7.0.0 of frontend library, date for this is unconfirmed