nhsuk-frontend icon indicating copy to clipboard operation
nhsuk-frontend copied to clipboard

Ctrl + Click on card doesn't open new tab

Open mikemonteith opened this issue 4 years ago • 14 comments

Bug Report

Ctrl + Click not respected on card components

What is the issue?

When ctrl + clicking on the body of a clickable card, the link navigates insside the current tab. It is expected that ctrl + click creates a new tab in most browsers.

What steps are required to reproduce the issue?

Ctrl + Click on a card. (examples here https://www.nhs.uk/pregnancy/)

N.B ctrl + click on the title of the card opens a new tab and navigates the current tab.

mikemonteith avatar Jun 22 '21 15:06 mikemonteith

This issue is caused by the fact that clickable cards use javascript to .click() on the <a> element when a card body is clicked on. Can we do something to support ctrl+click?

mikemonteith avatar Jun 22 '21 15:06 mikemonteith

Something like checking if the ctrl key is used may work (not tested):

card.addEventListener('click', (event) => {
    if (event.ctrlKey) {
        window.open(card.querySelector('a').getAttribute('href'));
    } else {
        card.querySelector('a').click();
    }
});

Although that doesn't factor in if the click is set up to do something else (highlighting target elements container fieldset/group for example). It may not work as expected on other platforms (e.g. MacOS).

SamNHS avatar Jun 22 '21 17:06 SamNHS

I think the root issue here is that we're faking a link when it's not semantically defined in the HTML. What if JS fails to load? We end up with a card that looks as if it's clickable when it's not.

mikemonteith avatar Jun 25 '21 08:06 mikemonteith

I think it may make sense to use the .js-enabled class for elements that are only clickable when JavaScript is enabled. The workaround now is to override the style:

.nhsuk-card--clickable {
  &:hover,
  &:active {
    cursor: auto;
  }
}

.js-enabled .nhsuk-card--clickable {
  &:hover,
  &:active {
    cursor: pointer;
  }
}

SamNHS avatar Jun 25 '21 10:06 SamNHS

It could be done without JS at all by positioning a pseudo element from inside the tag over the top of the whole card. So when you click on the card, you are clicking the underlying link element like this:

https://codepen.io/andymantell/pen/XWjXWzK

Pro: No JS needed - and all "normal" ways of clicking a link just work (i.e. Ctrl-Click, Middle click, Right click and select "open in new tab") Con: People cannot select and then copy+paste the contents of the card.

Interestingly, the current JS implementation already prevents users selecting the text because as soon as you mouseup on the card, it navigates away from the current page, even though I was trying to select the text, not click it!

card-select-bug

andymantell avatar Jun 25 '21 10:06 andymantell

I think the root issue here is that we're faking a link when it's not semantically defined in the HTML. What if JS fails to load? We end up with a card that looks as if it's clickable when it's not.

The JS loading styling issue could be resolved just by modifying how the styles are applied. Which probably should be done anyway.

Fenwick17 avatar Jun 25 '21 11:06 Fenwick17

Nomensa have a good blog post around clickable cards, different ways to do them and use them https://www.nomensa.com/blog/how-build-accessible-cards-block-links

@mikemonteith Users can still Ctrl + Click onto the link/title itself, as that still respects standard link behaviour.

Fenwick17 avatar Jun 25 '21 11:06 Fenwick17

@Fenwick17 Ctrl+click on the link/title itself causes a new tab and navigates the current tab. (Firefox) In Chrome it doesn't seem to open a new tab at all.

And anyway users shouldn't have to know where on the card they should click or not click if the entire thing is clickable

mikemonteith avatar Jun 25 '21 11:06 mikemonteith

I should be able to do some further testing on this and the example @andymantell provided next week. Adrien Roselli also has a good blog post regarding the CSS only implementation https://adrianroselli.com/2020/02/block-links-cards-clickable-regions-etc.html

Fenwick17 avatar Jun 25 '21 13:06 Fenwick17

I have done some testing on this with the CSS only solution, and a modified version of the current JS solution:

CSS Only

  • Generally works the same, but content within the card can't be highlighted at all (same as JS version)
  • No JS so bring performance improvements
  • 'Card with image' the image part of the card isn't clickable by default, so might need to restructure the HTML or CSS to make this work

JS solution

  • Can update the JS to look for metaKey (cmd) being pressed as an event on the card
card.addEventListener("click", (e) => {
  if (e.metaKey) {
    window.open(card.querySelector("a").getAttribute("href"));
  } else {
    card.querySelector("a").click();
  }
});
  • Unable to copy text due to click event
  • We can modify the CSS to be applied when JS loads, so they don't look clickable without JS

Fenwick17 avatar Jul 13 '21 13:07 Fenwick17

the JS solution would still not work when using a middle mouse button? And on windows, e.metaKey is the windows button, but it is the Ctrl button which is used to open new tabs.

andymantell avatar Jul 13 '21 13:07 andymantell

As for the CSS solution, with the following html:

<div class="nhsuk-card  nhsuk-card--clickable  ">     
  <img class="nhsuk-card__img" src="foo.jpg" alt="">  
  <div class="nhsuk-card__content">
    <h3 class="nhsuk-card__heading">
      <a class="nhsuk-card__link" href="https://www.nhs.uk/start4life/weaning">
        Is your baby ready for weaning?
      </a>
    </h3>
    <p class="nhsuk-card__description">Get expert advice, tips and healthy weaning recipes on Start4life.</p>
  </div>
</div>

position: relative on the whole .nhsuk-card would allow you to put the clickable area over the whole image, without restructuring the html? At the moment the .nhsuk-card__content seems to have position: relative on it but I can't see why. Unless anyone knows a reason for it being there, that could be removed, and allow the invisible clickable area to be extended.

andymantell avatar Jul 13 '21 13:07 andymantell

Generally I would be in favour of moving to the CSS only solution. position: relative can be added to the card and removed from the content without issue, or at least none I can spot.

The only real downside is the no text selection, which is semi possible with the JS implementation if you copy before mouseup.
But generally the text within these clickable cards shouldn't include anything that users may need to copy, such as reference numbers etc. Which will help reduce the issue.

Fenwick17 avatar Jul 13 '21 14:07 Fenwick17

Potentially related issue: https://github.com/nhsuk/nhsuk-frontend/issues/758

chrimesdev avatar Jul 26 '21 16:07 chrimesdev