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

Next/Previous Landmark should start from current focus #395

Closed
carmacleod opened this issue Jan 4, 2021 · 3 comments · Fixed by #409
Closed

Next/Previous Landmark should start from current focus #395

carmacleod opened this issue Jan 4, 2021 · 3 comments · Fixed by #409

Comments

@carmacleod
Copy link

Steps to see this issue:

  1. Go to https://carmacleod.github.io/playground/our-app/our-app.html
  2. Type Alt+Shift+N to go to the Banner landmark
  3. Type Alt+Shift+N to go to the Navigation landmark
  4. Now type Tab 6 times so that focus ends up on the "Click bait" link in the "Advertisement" Complimentary landmark
  5. Now type Alt+Shift+N again

At this point, I expected to go to "the Next landmark from where I am now", which would be the "Sign up for our Newsletter" Complimentary landmark.

Instead, I moved to the Main landmark, which is essentially moving backwards in the document, which is unexpected.
I don't think the user is expecting to go to "the Next landmark from where I was the last time I used Landmark navigation".
I'm pretty sure they will be expecting to move forward from the landmark that the currently-focused item is in.

Screen readers move to the next (and previous) landmark based on the current point of regard (i.e. focus or virtual cursor).
This can be seen with D and Shift+D in NVDA, using Tab to move focus, or Up/Down arrow to move the reading cursor;
or with R and Shift+R in JAWS.

@matatk
Copy link
Owner

matatk commented Jan 15, 2021

Thanks for this. I agree it should behave in the way you describe, as it does with screen readers.

I'm wondering: what happens if the user scrolls the page without moving the focus? My feeling is that someone who is browsing visually, but happens to be using the keyboard, would like the point of regard [what a neat expression :-)] to follow the scrolling, so it'd behave like the virtual cursor. Whilst this feels like a safe assumption, but I wonder if there are any keyboard-only power users I could ask first. Further, should the point of regard be taken as the centre of the screen, or somewhere nearer the top? The virtual cursor has the advantage of being precise.

From a technical/performance standpoint, this is an interesting problem to solve. Here are some thoughts...

  • We need to know in which landmark the focus currently lies (or at least which is the 'next' landmark, if the point of regard isn't currently inside of one). In fact, it really ties in with the ideas I had in A forest of trees! #389, so I ought to get on with testing the water there. Feels like it would be more efficient when that is implemented.

  • If handling scrolling as well as focus: is there a way to work out which takes precedence without having to listen to focus and scroll events? I can't think of one except maybe only take the scrolling position into account if the element with the focus is not currently visible. Will really have to test this on a wide range of sites to check it makes sense.

Feels like it's best to address the focus issue first and create a separate issue for the scroll one later if the need is felt.

@matatk matatk added this to the next milestone Jan 15, 2021
@carmacleod
Copy link
Author

Feels like it's best to address the focus issue first and create a separate issue for the scroll one later if the need is felt.

My feeling precisely when reading through your comment. Definitely go with the easy (focus) fix first, and decide later whether the other (scroll) is needed. Glad you ended up with the same conclusion! 😄

matatk added a commit that referenced this issue Feb 6, 2021
…ites (#408)

* Cache sites so that tests are more consistent over a short timespan at least.
* Add tests for navigation from the focused element (both forward and back) in support of #395.
* Numerous code clean-ups/improvements.
* Store totals/overall averages in the results (not sure how good of an idea this is).
@matatk
Copy link
Owner

matatk commented Feb 6, 2021

I've been testing some code for a little while, both in practice and via some improvements I made to the profiling script, and found that...

  • Each current navigation, ignoring focus, took about 0.003ms.
  • Walking the DOM from the focused element took about 3.2ms (three orders of magnitude slower—eek).
  • Using Node.compareDocumentPosition() however reduced this to just under 0.01ms (increasing the time navigation takes by a factor of about 3).

I do think this feature is necessary, so I think the slowdown is acceptable.

Thanks for the suggestion—it now seems so obvious this should've been how it behaved all along :-). That being said, it seems odd that this hasn't come up before. I've heard from some keyboard-centric (if not keyboard-only) users of the extension, and developers tend to like their keyboards too, but it does make me think maybe the extension is being used more by developers to check their sites than people visiting sites generally.

Along both of those lines, I've been working on promoting the extension a bit more, and now the profiling is improving, it'll be possible to work on more dev-centric features...

matatk added a commit that referenced this issue Feb 6, 2021
If there is a focused element in the document, navigate from its position when moving forward/backward between landmarks.

A test page for manual testing was added, and improvements to profiling allowed some good measurements. Was quite fun experimenting with more effective ways to do things.

Resolves #395
@matatk matatk removed this from the next milestone Feb 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants