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

Reach skip nav example #21376

Merged
merged 13 commits into from
Jun 1, 2020
Merged

Reach skip nav example #21376

merged 13 commits into from
Jun 1, 2020

Conversation

madalynrose
Copy link
Contributor

Description

Add an example project showcasing how to leverage @reach/skip-nav to create the ideal navigation focus experience based on research done by @marcysutton and Fable Labs. The updated recommendation was recently added to our blog post on client-side routing in PR #20540. Because this isn't achievable at the framework level, we should show users how to implement it in their own projects.

This is a cleaner version of #21107

@madalynrose madalynrose requested review from a team as code owners February 11, 2020 18:14
```javascript:layout.js
import { SkipNavLink, SkipNavContent } from "@reach/skip-nav"
import "@reach/skip-nav/styles.css" //this will show/hide the link on focus
;<>
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if for clarity for users, it would be easier to see this wrapped in a function called App?

@laurieontech
Copy link
Contributor

This is so awesome! A couple quick things.
When I tried running this I had to install gatsby-image. Looks like that might need to be included in the package.json?

I think the README has important information, but we should also include instructions for running the example. npm install, gatsby develop, etc.

blainekasten
blainekasten previously approved these changes Feb 11, 2020
Copy link
Contributor

@blainekasten blainekasten 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 to me!

@@ -0,0 +1,32 @@
# Using @reach/skip-nav
Copy link
Contributor

Choose a reason for hiding this comment

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

I still feel like this example needs to describe what it does, and the hook into client-side routing is significant to mention in addition to the "skip-nav" part.

@madalynrose madalynrose requested a review from a team March 25, 2020 19:30
@madalynrose
Copy link
Contributor Author

Okay I made a couple significant updates. Could I get another once-over from @gatsbyjs/learning?

@madalynrose
Copy link
Contributor Author

@marcysutton does this look mergeable?

Copy link
Contributor

@marcysutton marcysutton left a comment

Choose a reason for hiding this comment

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

Hey @madalynrose, I did another review of this. let me know what you think!

examples/using-reach-skip-nav/README.md Outdated Show resolved Hide resolved
examples/using-reach-skip-nav/README.md Outdated Show resolved Hide resolved
examples/using-reach-skip-nav/README.md Outdated Show resolved Hide resolved
examples/using-reach-skip-nav/README.md Outdated Show resolved Hide resolved
examples/using-reach-skip-nav/README.md Outdated Show resolved Hide resolved
examples/using-reach-skip-nav/README.md Show resolved Hide resolved
path: `${__dirname}/src/images`,
},
},
`gatsby-transformer-sharp`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do these Sharp/image plugins need to be in here?

* - `useStaticQuery`: https://www.gatsbyjs.org/docs/use-static-query/
*/

const Image = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this image component and dependencies be in here?

examples/using-reach-skip-nav/src/pages/index.js Outdated Show resolved Hide resolved
examples/using-reach-skip-nav/src/pages/self-care.js Outdated Show resolved Hide resolved
@madalynrose
Copy link
Contributor Author

Thanks for catching superfluous content @marcysutton! This should be good to go!

@ascorbic ascorbic added the status: needs docs review Pull request related to documentation waiting for review label May 12, 2020
@marcysutton
Copy link
Contributor

I think this example can be merged now, thanks so much for your work on it @madalynrose!

@marcysutton marcysutton merged commit 77d3f06 into master Jun 1, 2020
@delete-merged-branch delete-merged-branch bot deleted the reach-skip-nav-example branch June 1, 2020 18:23
axe312ger pushed a commit that referenced this pull request Jun 23, 2020
* add skip link example to simple starter

* update README

* add comments to gatsby-browser

* add gatsby-image to package.json

* update announcer live region to be polite

* update readme

* add cypress tests

* Apply language suggestions from code review

Co-Authored-By: Marcy Sutton <marcy@gatsbyjs.com>

* remove unneccessary image content from example

* remove unnecessary test

* run prettier on README.md

Co-authored-by: Marcy Sutton <marcy@gatsbyjs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs docs review Pull request related to documentation waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants