Adding .sr-only 'skip to content' link #161

MikeRogers0 · 2021-04-01T21:07:22Z

Issue https://github.com/zzak/sdoc/issues/158

Description

Further wins: Add skip navigation link

This adds a link to the top of each of the pages with "Skip to content" as the text which is activated by tabbing through the site, when clicks it, it should take the user to the #content <div>. The CSS is taken from Bootstrap 4.

Preview

Also video on YouTube demoing this

Testing

Skip to content

Chrome is the easiest to verify with, open the preview app & press "tab", a link saying "skip to content" . When you press "return" it should just the focus (and tab index) to the content.

Search

Press / to quickly start searching, this is similar to before where we had CTRL + S but is more inline with other big sites approaches.

SleeplessByte

🥳

Why #bodyContent and not #content?

It might also be semantically wise to mark the #content div as main instead. Landmark roles make people who use assistive technology, happy!

(Needs the remove tab hijack PR in order to test well)

MikeRogers0 · 2021-04-02T23:21:22Z

Why #bodyContent and not #content?

Not sure, but I do prefer #content. I updated the links to point to #content over #bodyContent

It might also be semantically wise to mark the #content div as main instead.

Updated :)

SleeplessByte · 2021-04-02T23:31:31Z

Not sure, but I do prefer #content. I updated the links to point to #content over #bodyContent

~~I think there's already id="content" inside some/all of the _context partials! That's what confused the "double content" layering for me. Want to confirm? Want me to look it up for you?~~

Lol. The last commit didn't load for me! But now it has, and you figured it out :P.

p8 · 2021-04-08T17:05:03Z

@MikeRogers0 could you squash the commits?

SleeplessByte · 2021-04-08T17:15:38Z

(Apologies if the intention is not to squash then merge, but if the intention is squash-then-merge, GitHub provides this functionality on merge)

MikeRogers0 · 2021-04-08T17:19:25Z

@p8 - Done :)

But also +1 on "Squash and Merge", I use that on all my projects & I love it!

p8 · 2021-04-12T06:12:15Z

@SleeplessByte @MikeRogers0 Thanks 😄
I know about the squash button.
I used it on zzak@0e5a715
But I prefer the contributor squashing the commits as they have a better understanding of what can be squashed.

SleeplessByte

I don't know for sure if this is the syntax required to disable turbolinks, but see the behaviour on those two links.

Click the search box
Shift + Tab (reverse tab) should show Skip to content
Follow the skip to content link
Press Tab

In the correct behaviour it now focuses the first link on the right hand side. Here it worked. In the incorrect case it now shows the skip to content link again, which means that #content wasn't focussed, but body was.

lib/rdoc/generator/template/rails/class.rhtml

lib/rdoc/generator/template/rails/file.rhtml

lib/rdoc/generator/template/rails/index.rhtml

SleeplessByte · 2021-04-13T21:35:06Z

Weird. I looked up the docs and data-turbolinks="false" seems correct, but I can still see it trigger turblinks when triggering the #content link, and I don't quite know why.

MikeRogers0 · 2021-04-13T23:18:34Z

@SleeplessByte That's a really odd issue! But I figured out the cause!

At first I thought it might be the older version of Turbolinks, then I thought the inline JavaScript in the <head> element was the cause, now I'm a bit stumped as to what's causing turbolinks to do that.

I'm going to look more closely with fresh eyes tomorrow :)

zzak · 2021-04-14T04:33:37Z

@MikeRogers0 feel free to ping me too if you need or want a review once this is ready

MikeRogers0 · 2021-04-25T18:43:45Z

I've not forgotten about this PR, I'm still trying to figure out why clicking the anchor link is trigging a full page refresh in Turbolinks :)

MikeRogers0 · 2021-05-19T16:48:07Z

lib/rdoc/generator/template/rails/_head.rhtml

-    panel.toggle(<%= tree_keys %>);
-  }
-})
-</script>


This was adding a new listener each page view, so I moved it into the main.js, passing in the contextual data via meta tags.

lib/rdoc/generator/template/rails/index.rhtml

lib/rdoc/generator/template/rails/resources/css/main.css

MikeRogers0 · 2021-05-19T17:02:53Z

lib/rdoc/generator/template/rails/resources/js/searchdoc.js

@@ -63,6 +63,7 @@ Searchdoc.Navigation = new function() {
                }
                break;
            case 13: //Event.KEY_RETURN:
+                if(e.target.dataset["turbolinks"]) { break; }


This is the fix I had for the pages reloading when the "skip to content" was clicked via a keyboard user. Pretty much, before if a user was navigating via the keyboard the JS would think they're interacting with the search, and it caused a re-render.

Ah. Yes. Turbolinks does stuff like that :/

Good find 😄

MikeRogers0 · 2021-05-19T17:27:12Z

I figured out what was causing the double navigating issue. @SleeplessByte could you verify this solves the issue? :)

SleeplessByte · 2021-05-19T18:11:54Z

It seems to have solved everything except that I now don't know how to search if I'm only using my keyboard 😅 . Is there a shortcut to press?

MikeRogers0 · 2021-05-19T18:57:03Z

@SleeplessByte - Oh gosh, yeah I can imagine that would be annoying. Do you think a "Jump to Search" link/label would solve that?

p8 · 2021-05-19T19:00:35Z

@MikeRogers0 Github and Twitter use "/" for search (as do a lot of commandline apps).

p8 · 2021-05-19T19:05:25Z

The current shortcut for search is CTRL+s

SleeplessByte · 2021-05-19T19:18:44Z

I would probably add this to the search placeholder (search, type / to start searching) and perhaps add it to a sr-only thing right after the skip to navigation. So people can skip the search message, but could still reach it 😬

MikeRogers0 · 2021-05-19T20:00:17Z

I added support for the "/", while also keeping the previous ctrl+s, along with the "Skip to Search" link.

p8 · 2021-05-19T20:03:57Z

@MikeRogers0 Weird. If I'm opening the latest netlify preview the search input is focussed again by default.

MikeRogers0 · 2021-05-19T20:08:07Z

@p8 - I did a hard refresh & I'm seeing that also 🤔

MikeRogers0 · 2021-05-19T22:23:33Z

lib/rdoc/generator/template/rails/_head.rhtml

-  if(!$('#panel .tree ul li').length) {
-    $('#links').hide();
-    var panel = new Searchdoc.Panel($('#panel'), search_data, tree, '<%= rel_prefix %>/');
-    $('#search').focus();


This LOC was causing the refocus when I moved this over to be in main.js. I removed the focus @p8 in the latest version.

After adding these, I ran into some fun JS related errors where we'd benefit moving things, along with the SearchDoc interfering with Turbolinks. Setting sidebar to be a <nav> as that's its purpose - Let's see how that helps with SEO. Hinting more clearly we have a key command for quick searching (and adding support for / )

MikeRogers0 · 2021-05-19T23:19:27Z

@p8 - Solved it! We had a $("#search").focus() in there from before we didn't need any more.

I think this is ready now.

zzak · 2021-05-20T00:52:48Z

@MikeRogers0 Can you provide some instructions on how to test this? I haven't been following too closely and there's a bunch of different screenshots :)

MikeRogers0 · 2021-05-20T01:37:37Z

@zzak I've updated the PR description to be a bit more clearer https://github.com/zzak/sdoc/pull/161#issue-607671522 :) I also just recorded a video on what it does ( https://www.youtube.com/watch?v=RbXg8BLSDpY ) :)

SleeplessByte · 2021-05-20T10:54:38Z

(Just a reminder why I mentioned skip-to-content was a quick win is to skip over this section, which sometimes is really big, especially in the rails docs)

)

SleeplessByte · 2021-05-20T10:55:02Z

@MikeRogers0 I just tried it out and it's ✨ wonderful!

p8 · 2021-05-20T16:31:27Z

@SleeplessByte I think collapsing that header would be useful as well.

p8

I'll wait for @zzak but this looks great to me.
After it's merged I can do a new release and PR to Rails.

zzak · 2021-05-20T21:46:52Z

@MikeRogers0 Thanks for your hard work on this and the YT video was very helpful!

@p8 Feel free to merge + release at your convenience 🙇

p8 · 2021-05-21T08:16:08Z

I've released 2.2.0 and made a PR for Rails:
rails/rails#42265
Thanks everyone!

MikeRogers0 marked this pull request as ready for review April 1, 2021 21:47

SleeplessByte approved these changes Apr 2, 2021

View reviewed changes

MikeRogers0 mentioned this pull request Apr 2, 2021

Add Netlify preview for Pull Requests #162

Closed

MikeRogers0 mentioned this pull request Apr 13, 2021

Adding netlify.toml - We can use it for PR previews #163

Merged

SleeplessByte reviewed Apr 13, 2021

View reviewed changes

lib/rdoc/generator/template/rails/class.rhtml Outdated Show resolved Hide resolved

lib/rdoc/generator/template/rails/file.rhtml Outdated Show resolved Hide resolved

lib/rdoc/generator/template/rails/index.rhtml Outdated Show resolved Hide resolved

MikeRogers0 marked this pull request as draft April 14, 2021 08:12

MikeRogers0 commented May 19, 2021

View reviewed changes

lib/rdoc/generator/template/rails/index.rhtml Show resolved Hide resolved

MikeRogers0 commented May 19, 2021

View reviewed changes

lib/rdoc/generator/template/rails/resources/css/main.css Show resolved Hide resolved

MikeRogers0 commented May 19, 2021

View reviewed changes

MikeRogers0 marked this pull request as ready for review May 19, 2021 17:17

MikeRogers0 commented May 19, 2021

View reviewed changes

MikeRogers0 marked this pull request as draft May 19, 2021 23:04

MikeRogers0 marked this pull request as ready for review May 19, 2021 23:18

p8 approved these changes May 20, 2021

View reviewed changes

zzak approved these changes May 20, 2021

View reviewed changes

p8 merged commit ba35851 into rails:master May 21, 2021

Adding .sr-only 'skip to content' link #161

Adding .sr-only 'skip to content' link #161

Conversation

MikeRogers0 commented Apr 1, 2021 • edited Loading

Description

Preview

Testing

Skip to content

Search

SleeplessByte left a comment • edited Loading

Choose a reason for hiding this comment

MikeRogers0 commented Apr 2, 2021 • edited Loading

SleeplessByte commented Apr 2, 2021 • edited Loading

p8 commented Apr 8, 2021

SleeplessByte commented Apr 8, 2021

MikeRogers0 commented Apr 8, 2021

p8 commented Apr 12, 2021

SleeplessByte left a comment

Choose a reason for hiding this comment

SleeplessByte commented Apr 13, 2021

MikeRogers0 commented Apr 13, 2021

zzak commented Apr 14, 2021

MikeRogers0 commented Apr 25, 2021

MikeRogers0 May 19, 2021

Choose a reason for hiding this comment

MikeRogers0 May 19, 2021

Choose a reason for hiding this comment

SleeplessByte May 19, 2021

Choose a reason for hiding this comment

p8 May 19, 2021

Choose a reason for hiding this comment

MikeRogers0 commented May 19, 2021

SleeplessByte commented May 19, 2021

MikeRogers0 commented May 19, 2021

p8 commented May 19, 2021

p8 commented May 19, 2021

SleeplessByte commented May 19, 2021

MikeRogers0 commented May 19, 2021

p8 commented May 19, 2021

MikeRogers0 commented May 19, 2021

MikeRogers0 May 19, 2021

Choose a reason for hiding this comment

MikeRogers0 commented May 19, 2021 • edited Loading

zzak commented May 20, 2021

MikeRogers0 commented May 20, 2021

SleeplessByte commented May 20, 2021

SleeplessByte commented May 20, 2021

p8 commented May 20, 2021

p8 left a comment • edited Loading

Choose a reason for hiding this comment

zzak commented May 20, 2021

p8 commented May 21, 2021

MikeRogers0 commented Apr 1, 2021 •

edited

Loading

SleeplessByte left a comment •

edited

Loading

MikeRogers0 commented Apr 2, 2021 •

edited

Loading

SleeplessByte commented Apr 2, 2021 •

edited

Loading

MikeRogers0 commented May 19, 2021 •

edited

Loading

p8 left a comment •

edited

Loading