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.

Sign up for GitHub

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

Jump to bottom

Add helper: click_govuk_link #6

Merged
merged 6 commits into from
Sep 6, 2023
Merged

Add helper: click_govuk_link #6

merged 6 commits into from
Sep 6, 2023

Conversation

frankieroberto
Copy link
Contributor

This is a drop-in replacement for the capybara click_link helper, but also checks that:

  • there aren’t 2 or more links with the same link text
  • the link has the govuk-link class (or another link class from the GOV.UK Design System)
  • the link text isn't ambiguous like "Change"
  • the full link text is specified (including any visually-hidden text) rather than a partial match

@frankieroberto
Add helper: click_govuk_link
9629692 
This is a drop-in replacement for the capybara `click_link` helper, but adds some additional checks.
@frankieroberto frankieroberto marked this pull request as draft September 4, 2023 22:35
@frankieroberto
Copy link
Contributor Author

@paulrobertlloyd @peteryates @joelanman can you think of any other link-related accessibility or usability checks that could be included?

@joelanman
Copy link

joelanman commented Sep 5, 2023 •
edited
Loading

  • I think they can have same link text if same href
  • if it has govuk-button class, that it also has the right role and data attributes
  • maybe a warning if the _target is blank?
  • that it doesnt have a button inside it

@paulrobertlloyd
Copy link
Contributor

paulrobertlloyd commented Sep 5, 2023 •
edited
Loading

Building on @joelanman’s suggestion, how about a warning if the _target is blank but the link text doesn't contain the words ‘new tab’?

@frankieroberto
Copy link
Contributor Author

  • I think they can have same link text if same href

Oh, I wasn’t sure that was allowed. Will check.

  • if it has govuk-button class, that it also has the right role and data attributes

I was planning on adding a separate click_govuk_button helper which would handle both real buttons and links styles as buttons. But I think it’s a good idea to detect them within click_govuk_link and raise an error suggesting people switch to the other helper. 👍

  • maybe a warning if the _target is blank?

👍

  • that it doesnt have a button inside it

@joelanman could you explain this one? I’ve never seen a button within a link… 🤔

@joelanman
Copy link

ha there's no explanation - some people do it!

@frankieroberto frankieroberto marked this pull request as ready for review September 6, 2023 16:22
@frankieroberto
Copy link
Contributor Author

@joelanman @paulrobertlloyd thanks! I've incorporated all your suggestions.

You can see the documentation page for this helper here: https://x-govuk.github.io/govuk-rspec-helpers/click-govuk-link/

We can continue to add checks in future if needed!

@frankieroberto frankieroberto merged commit c294822 into main Sep 6, 2023
@frankieroberto frankieroberto deleted the add-click-govuk-link branch September 6, 2023 16:49
@paulrobertlloyd
Copy link
Contributor

You can see the documentation page for this helper here: https://x-govuk.github.io/govuk-rspec-helpers/click-govuk-link/

Ooh, documentation. Sounds like somebody wants a homepage illustration, right? Right!?

@frankieroberto frankieroberto mentioned this pull request Sep 6, 2023
Merged
@peteryates
Copy link
Member

I like this ❤️

One thing that is probably worth checking for is when target: "_blank" there's a rel: "noreferrer noopener" present too?

Many of these will be solved by the new link helpers coming in GOV.UK Components v5.

@frankieroberto
Copy link
Contributor Author

@peteryates good shout! I've added that as an issue here: #14 - will open a PR shortly.

frankieroberto added a commit that referenced this pull request Sep 13, 2023
@frankieroberto
Add click_govuk_button helper (#8)
eca60b5 
Following the `click_govuk_link` helper added in #6, this does the same
but for buttons.

It will work with both actual `<button>`s, and with links styled as
buttons – on the basis that the tests should reflect how the interfaces
appears to users, rather than the underlying implementation.

The helper currently does these checks:

* full text of the button is specified, including any visually-hidden
text
* the button text is unique within the page
* the `gov-button` class is added
* `data-module="govuk-button"` is added (this is used by javascript
which adds some accessibility enhancements)
* if it’s a link styled as a button, then `role="button"` is present
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peteryates peteryates Awaiting requested review from peteryates

@paulrobertlloyd paulrobertlloyd Awaiting requested review from paulrobertlloyd

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@frankieroberto @joelanman @paulrobertlloyd @peteryates