Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ctrl + Click on card doesn't open new tab #749

Closed
mikemonteith opened this issue Jun 22, 2021 · 14 comments · Fixed by #762
Closed

Ctrl + Click on card doesn't open new tab #749

mikemonteith opened this issue Jun 22, 2021 · 14 comments · Fixed by #762

Comments

@mikemonteith
Copy link
Contributor

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
Copy link
Contributor Author

mikemonteith commented Jun 22, 2021

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?

@SamNHS
Copy link

SamNHS commented Jun 22, 2021

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).

@mikemonteith
Copy link
Contributor Author

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.

@SamNHS
Copy link

SamNHS commented Jun 25, 2021

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;
  }
}

@andymantell
Copy link
Contributor

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

@Fenwick17
Copy link
Contributor

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
Copy link
Contributor

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.

@mikemonteith
Copy link
Contributor Author

@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

@Fenwick17
Copy link
Contributor

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
Copy link
Contributor

Fenwick17 commented Jul 13, 2021

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

@andymantell
Copy link
Contributor

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
Copy link
Contributor

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.

@Fenwick17
Copy link
Contributor

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.

@chrimesdev
Copy link
Member

Potentially related issue: #758

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants