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

Skip links component #2356

Closed
myasonik opened this issue Sep 20, 2019 · 11 comments · Fixed by #2976
Closed

Skip links component #2356

myasonik opened this issue Sep 20, 2019 · 11 comments · Fixed by #2976
Assignees
Labels

Comments

@myasonik
Copy link
Contributor

myasonik commented Sep 20, 2019

Proposal

<EuiSkipLink destination="{url}">

A skip link is an anchor with a target on the same page as the user to let them skip ahead over some content that's usually repeated across pages and particularly tab heavy. Most commonly, these will be used as the first tabbable element on a page to let users skip over the page navigation to go to the main content.

Future enhancements

To start off, I think a single element that's visually hidden that appears when receives focus is all we need and works both as first tab stop on a page to skip to main content but also anywhere on a page where a skip link might be helpful.

In the future, we might want to create a more elaborate skip link for a first tab stop on a page which a dedicated component leaves the door open for. However, I don't imagine this option being particularly urgent or high value for us right now.

Examples

Github has a basic, bare bones example:
github

Starbucks has a simple, well styled, and somewhat more advanced versions:
starbucks

Facebook really stretches the meaning of a skip link which a whole skip link section which, I found, frankly hard to use:
facebook

Alternatives

  1. We build this in Kibana
    I don't think this is a good option because I think EUI is a better place to control the design of skip links. Also, as all apps, not just Kibana, should use skip links, it'd probably lead to code duplication.
  2. Instead of a new component, make it a property on EuiLink
    Though these render as an <a> tag, they're visually, often, similar to buttons which can lead to developer confusion on which component they need to reach for.
  3. Instead of a new component, make it a property on EuiButton
    Though these often are visually similar to buttons, they don't necessarily have to be and the have the semantics of an <a> tag which can lead to developer confusion on which component they should reach for.

More words by others

@ryankeairns
Copy link
Contributor

The Kibana Core UI team has an issue to add a 'Skip to content' link which is a rather fundamental accessibility feature, so I'm considering tackling this issue. First, a few questions :)

To start off, I think a single element that's visually hidden that appears when receives focus is all we need and works both as first tab stop on a page to skip to main content but also anywhere on a page where a skip link might be helpful.

This seems sufficient for addressing the immediate need and is likely all we would need for some time. Given that...

We build this in Kibana
I don't think this is a good option because I think EUI is a better place to control the design of skip links. Also, as all apps, not just Kibana, should use skip links, it'd probably lead to code duplication.

This makes sense to me given it should be used by Kibana, Cloud, Dream Machine, etc.

Presuming others agree with the above, then my question would be how should we build this in EUI? Michail lists some alternatives above and I can see it either being a new type on EuiLink or, given its simplicity, creating a new component.

What do you all think?

@cchaos
Copy link
Contributor

cchaos commented Mar 2, 2020

I would propose a different solution as I think about how consumers might try to find, or consume this type of component.

We already have EuiScreenReaderOnly which visually hides its children. We can then add an option to make its children visible on focus. This way it's a much more generalized component for any other instance where focus visibility is needed.

In the docs, we can showcase an example for using it as a "skip links" button specifically and the EuiNavDrawer can offer a quick prop like skipLinkDestination for adding this utility.

@myasonik
Copy link
Contributor Author

myasonik commented Mar 2, 2020

If we repurpose EuiScrenReaderOnly for this, which I do admit is quite a nice extension, I wonder if it would need a rename though? As this extended the scope of the component quite a bit... (With visible on-focus children it's expressly now not for "screen readers only".)

@cchaos
Copy link
Contributor

cchaos commented Mar 2, 2020

Just remember that because of the wide usages of EUI and such a major component as EuiScreenReaderOnly, name changes is a huge breaking change and so you need to truly consider if it's a necessary path.

That being said, for this particular component I do still think the current name is appropriate since only keyboard users can access focus only elements and, right or wrong, I think most consumers equate this functionality with those who use assistive technology such as screen readers.

@snide
Copy link
Contributor

snide commented Mar 2, 2020

Can always do both. Can just be a prop on EuiScreenReaderOnly and then have a separate component EuiOnFocusLinks or something that wraps it, automatically applies the prop and also takes an array of links.

@ryankeairns
Copy link
Contributor

+1 the 'do both' approach. I've got something close to working on the Kibana side that I can convert over.

@ryankeairns
Copy link
Contributor

To keep in in the same home, I can add an example to Utilities > Accessibility page as opposed to having it appear as a new component in the main list. Does that work? It may make sense on the nav/header examples, as Caroline noted, or we could use it on the EUI docs site itself :)

@cchaos
Copy link
Contributor

cchaos commented Mar 2, 2020

I"m fine with a more component specific component that extends EuiScreenReaderOnly. But just to be clear in case someone take this up, EuiOnFocusLinks I think is still too specific. Truly, what we're trying to accomplish is just make content invisible until focused. The content can be anything.

I'd recommend something like EuiKeyboardFocusOnly.

@cchaos
Copy link
Contributor

cchaos commented Mar 2, 2020

Errr, I re-read that and the naming is still wrong lol, more like EuiVisibleOnFocusOnly... ugh naming is hard. But it needs to be this generic.

@snide
Copy link
Contributor

snide commented Mar 2, 2020

That makes sense.

@myasonik
Copy link
Contributor Author

myasonik commented Mar 2, 2020

Caroline and I talked off GitHub just cuz back-and-forth can be tricky here. Anyway...

It seemed like having EuiScreenReaderOnly with a prop of showOnFocus and then having EuiVisibleOnFocusOnly seemed like too thin of a wrapper and didn't gain us much because we'd still have to document a pattern of EuiVisibleOnFocusOnly with EuiButton to make a skip link.

So we're proposing to go instead with EuiScreenReaderOnly with the prop and an EuiSkipLink component. This reduces a burden of documentation on EUI and makes it easier for implementing developers looking for quick and easy defaults (while also maintaining the lowest-level components for unique situations).

@cchaos chime in if I got anything wrong!

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

Successfully merging a pull request may close this issue.

4 participants