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

Add support for multiple skip links in header #189

Merged
merged 4 commits into from
Feb 16, 2021

Conversation

edwardhorsford
Copy link
Contributor

@edwardhorsford edwardhorsford commented Feb 11, 2021

Our service has a number of filters you can apply to filter a group of records. We recently used the accessibility personas to review these pages - and felt it would be good to provide a way of skipping past the filters to the results.

In this pr I'm experimenting by adding a second skip link in to the header. This uses the newish :focus-within property to show the skip-link div if either of the children links are focussed. This should gracefully degrade in browsers that don't support the property to just show two skip links in turn.

The css is mostly copied styles from the govuk-skip-link component, but applied to the header instead. If the browser supports :focus-within I then remove some of the default styles from the skip links themselves.

Video:

dual-skip-links

Without support for the property:

double-skip-link

@muqi1 muqi1 temporarily deployed to register-pro-multiple-s-kqbumi February 11, 2021 13:08 Inactive
@edwardhorsford edwardhorsford temporarily deployed to register-pro-multiple-s-kqbumi February 11, 2021 13:18 Inactive
@edwardhorsford edwardhorsford temporarily deployed to register-pro-multiple-s-kqbumi February 11, 2021 16:43 Inactive
@@ -4,6 +4,24 @@
{% set backLink = 'false' %}
{% set navActive = "records" %}

{% block skipLink %}

<div class="app-skip-link--container">
Copy link
Contributor

@jesseyuen jesseyuen Feb 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider indenting app-skip-link--container

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! fixed.

<span class="app-skip-link--item">
{{ govukSkipLink({
href: '#records-list',
text: 'Skip to results' if selectedFilters else 'Skip to records'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@@ -0,0 +1,99 @@
$important: true;

// This is basically a copy of the Design System govuk-visually-hidden-focusable mixin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to your code but I find a bunch of the design system css so hard to read. 🧐

Copy link
Contributor

@jesseyuen jesseyuen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great Ed. I gave it a test in a legitimate ie11 instance too and the fallback works well.

@edwardhorsford edwardhorsford temporarily deployed to register-pro-multiple-s-kqbumi February 16, 2021 10:21 Inactive
@edwardhorsford edwardhorsford merged commit fa56bf6 into master Feb 16, 2021
@edwardhorsford edwardhorsford deleted the multiple-skip-links branch February 16, 2021 10:22
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 this pull request may close these issues.

3 participants