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 accessibility documentation #5856

Merged
merged 5 commits into from
Aug 16, 2022
Merged

Add accessibility documentation #5856

merged 5 commits into from
Aug 16, 2022

Conversation

geoffrich
Copy link
Member

@geoffrich geoffrich commented Aug 8, 2022

Add docs around SvelteKit's accessibility features and what developers need to do to play nicely with them.

I focused these docs on SvelteKit-specific concerns (route announcements, focus management) instead of general a11y practices (e.g. skip link, button vs link, etc). In the future, a "Guides" section on learn.svelte.dev may be a good place to go deeper into general a11y best practices, or integrated into the official SvelteKit tutorial.

I'm looking for feedback on prose, accuracy of information, and thoughts on anything else we want to include (or exclude).

Leaving in draft for now until the dust settles on the big routing changes.

Prior art:

Context around route announcement/focus management: #307


Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@changeset-bot
Copy link

changeset-bot bot commented Aug 8, 2022

⚠️ No Changeset found

Latest commit: 44d70f1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR


You can also programmatically navigate to a different page using the [`goto`](/docs/modules#$app-navigation-goto) function. By default, this will have the same client-side routing behavior as clicking on a link. However, `goto` also accepts a `keepfocus` option that will preserve the currently-focused element instead of resetting focus. If you enable this option, make sure the currently-focused element still exists on the page after navigation.

### Lang attribute
Copy link
Member Author

Choose a reason for hiding this comment

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

It might also be good to show how to dynamically update the lang attribute for multi-lang sites - see #3091 (comment)

Copy link

Choose a reason for hiding this comment

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

NB: the example in #3091 (comment) refers to transformPage() but this was replaced with transformPageChunk() in #5657.

Copy link

@MelSumner MelSumner left a comment

Choose a reason for hiding this comment

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

@geoffrich This is really useful information. It identifies what's available, what users need to do themselves, and reliable links for additional info. 👍

});
```

You can also programmatically navigate to a different page using the [`goto`](/docs/modules#$app-navigation-goto) function. By default, this will have the same client-side routing behavior as clicking on a link. However, `goto` also accepts a `keepfocus` option that will preserve the currently-focused element instead of resetting focus. If you enable this option, make sure the currently-focused element still exists on the page after navigation.

Choose a reason for hiding this comment

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

[question, suggestion] Is there a fallback if the element no longer exists? Might be useful to let devs know what the outcome would be, either way. 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

There's no fallback element -- when keepfocus is set, SvelteKit won't call focus at all. So if the element is removed, focus will be lost just as if you removed the currently focused element in vanilla JS. I can try to clarify this part a bit. Maybe add something like:

If the element no longer exists, the user's focus will be lost.

@accuser
Copy link

accuser commented Aug 9, 2022

@geoffrich this is a great start, and the snippets you have included are really helpful.

My only caution is that getting a11y right requires a lot more thought, and I wonder if this needs to be more explicit in the documentation? i.e., whilst Svelte/SvelteKit provides some really good helpers, on their own they don't make an a11y app.

You're already thinking about this with the proposed 'Guides' section on learn.svelte.dev, but helping people be aware of the built-in capabilities as well as understand that these alone don't make an accessible app would be a really open approach to building a better AX.

@benmccann benmccann added the documentation Improvements or additions to documentation label Aug 9, 2022
Comment on lines +9 to +10
We recognize that accessibility can be hard to get right. If you want to suggest improvements to how SvelteKit handles accessibility, please [open a GitHub issue](https://github.com/sveltejs/kit/issues).

Copy link
Member

Choose a reason for hiding this comment

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

I'm not really sure we need this sentence. It could apply to any section as we want people to open issues about issues they encounter anywhere.

Suggested change
We recognize that accessibility can be hard to get right. If you want to suggest improvements to how SvelteKit handles accessibility, please [open a GitHub issue](https://github.com/sveltejs/kit/issues).

Choose a reason for hiding this comment

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

[comment] I thought this bit was thoughtful; it's signaling to the accessibility community that their feedback is welcome, which is a welcome exception in tech (by large, over the last 30 years). The sentence may not be a technical necessity but it might be a community necessity. Just a perspective to consider. 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

I can go either way on this, but Melanie captures why I included it in the first place. It emphasizes that we want to be receptive to a11y suggestions and improve as needed.

Copy link
Member

Choose a reason for hiding this comment

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

Agree with keeping it

@geoffrich
Copy link
Member Author

My only caution is that getting a11y right requires a lot more thought, and I wonder if this needs to be more explicit in the documentation? i.e., whilst Svelte/SvelteKit provides some really good helpers, on their own they don't make an a11y app.

@accuser would something to this effect in the intro be what you're looking for? "Keep in mind that while SvelteKit aims to provide an accessible foundation, you are still responsible for making sure your application code is accessible. If you're new to accessibility, see the "Further Reading" section of this guide for additional resources."

I had something similar to that in one of my drafts, but it got lost along the way.

@geoffrich
Copy link
Member Author

The big +page PR merged, so this is ready for review now.

@geoffrich geoffrich marked this pull request as ready for review August 16, 2022 14:47
Copy link
Member

@Rich-Harris Rich-Harris left a comment

Choose a reason for hiding this comment

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

this is great! i added one very small suggestion, but either way this gets a hearty 👍 from me

@Rich-Harris Rich-Harris merged commit 63da9df into master Aug 16, 2022
@Rich-Harris Rich-Harris deleted the grich/a11y-docs branch August 16, 2022 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants