Should also update focus

[danielnixon]

Try opening this (non-SPA) link in your browser and then pressing tab key:

https://en.wikipedia.org/wiki/Firefox#Performance

(Focus will move to the first link after the #Performance ID)

Compare that to: http://react-router-hash-link.rafrex.com/bar#section-two

(which doesn't move focus)

[danielnixon]

It's (sadly) not as simple as calling focus() and calling scrollIntoView().

Some links:

• https://css-tricks.com/smooth-scrolling-accessibility/#article-header-id-4
• https://developer.paciellogroup.com/blog/2014/08/using-the-tabindex-attribute/
• https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/focus#Browser_compatibility
• Disable / customize scrolling behavior of Element.focus whatwg/html#834
• https://stackoverflow.com/questions/4963053/focus-to-input-without-scrolling/6610501
• https://github.com/calvellido/focus-options-polyfill

[chadlavi]

Anyone have recommendations for a workaround on this? This makes any app that uses this library break WCAG accessibility standards.

[rafgraph]

It’s not so much an issue with this library as it is an issue with the native dom api scrollIntoView() which unfortunately doesn’t move focus with it, not sure why (oversight or intentional).

[chadlavi]

Here's my sorta hack for doing this:

import * as React from 'react'
import {HashLink, HashLinkProps} from 'react-router-hash-link'

export const AccessibleHashlink = (props: HashLinkProps): JSX.Element => {
  const focusTarget = (): void => {
    const route = props.to
    if (typeof route === 'string') {
      const hashes = route.split('#')
      if (hashes.length > 1) {
        const id = hashes.pop()
        if (id) {
          const h = `#${id}`
          setTimeout(() => {
            const e = document.querySelector(h)
            const p = e?.parentElement
            if (p) {
              const a = document.createElement('span')
              const unique = uuid()
              a.setAttribute('id', `${h}-hack`)
              a.setAttribute('tabIndex', '0')
              p.insertBefore(a, e)
              a.focus()
              a.addEventListener('blur', () => {
                p.removeChild(a)
              })
            }
          }, 1)
        }
      } else {
        window.scrollTo({top: 0})
      }
    }
  }

  return (
    <HashLink
      {...props}
      onClick={focusTarget}
    />
  )
}

This creates an invisible focusable span just before the element with the ID you're jumping to and holds focus. When the span loses focus, it is removed from the DOM.

[rafgraph]

I added this functionality to v2.2.0, unfortunately it doesn't work in Safari (desktop and iOS), but I figure it's better to have it working in most browsers than no browsers.

[danielnixon]

For Safari (and IE) considerations, maybe have a look at this: https://github.com/oaf-project/oaf-side-effects/blob/master/src/index.ts#L286-L359

export const focusElement = async (
  element: Element,
  preventScroll = false,
): Promise<boolean> => {
  // See: https://developer.paciellogroup.com/blog/2014/08/using-the-tabindex-attribute/
  // See: https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/focus#Browser_compatibility
  // See: https://github.com/whatwg/html/issues/834
  // See: https://stackoverflow.com/questions/4963053/focus-to-input-without-scrolling/6610501

  if (!(element instanceof HTMLElement || element instanceof SVGElement)) {
    console.warn(
      // TODO fix this
      // eslint-disable-next-line @typescript-eslint/no-base-to-string
      `Cannot focus element. Element [${target.toString()}] is not an HTMLElement or SVGElement.`,
    );
    return Promise.resolve(false);
  }

  try {
    // Set tabindex="-1" if necessary.
    // TODO avoid setting tabindex when we're confident we don't need to?
    if (!element.hasAttribute("tabindex")) {
      element.setAttribute("tabindex", "-1");
      // We remove tabindex after blur to avoid weird browser behavior
      // where a mouse click can activate elements with tabindex="-1".
      const blurListener = (): void => {
        element.removeAttribute("tabindex");
        element.removeEventListener("blur", blurListener);
      };
      element.addEventListener("blur", blurListener);
    }

    if (preventScroll) {
      // preventScroll has poor browser support, so we restore scroll manually after setting focus.
      // TODO detect if browser supports preventScroll and avoid `withRestoreScrollPosition`
      // shenanigans if so.
      await withRestoreScrollPosition(() => {
        try {
          element.focus({ preventScroll: true });
        } catch {
          // If focus() with options throws, fall back on calling focus() without any arguments.
          element.focus();
        }
      });
    } else {
      // Avoid passing anything to focus() (when we can) to maximize browser compatibility.
      element.focus();
    }

    return document.activeElement === element;
  } catch (e: unknown) {
    // Apparently trying to focus a disabled element in IE can throw.
    // See https://stackoverflow.com/a/1600194/2476884
    console.error(e);
    return false;
  }
};

[rafgraph]

So the tradeoff is: 1) leave focus on the element that is scrolled to until a user event blurs it (as is the case in the above code), or 2) immediately blur the element after calling focus.

With 1, you end up with focus styles on an element that might never have otherwise had focus styles, so it doesn't move focus silently (as is the case in the above code).

With 2, it moves focus silently but unfortunately calling blur immediately after focus causes focus to reset to where it was previously in Safari (in Chrome it works fine).
See: https://github.com/rafgraph/react-router-hash-link/blob/main/src/index.js#L36-L45

I chose way 2 as the default behavior, but maybe it should be way 1? Or maybe make way 1 an option with prop focusScrolledToElement?

[danielnixon]

you end up with focus styles on an element that might never have otherwise had focus styles

There's some interesting debate on that question at w3c/wcag#1001

immediately blur the element after calling focus

I think this defeats the purpose of setting focus, so one may as well have not set focus at all.

[rafgraph]

Calling focus and then blur moves the focus (except in safari), so it is very useful. If the user presses the tab key the element that receives focus is the one after where focus/blur was called. Try it out in the demo site.. start at section one, click to scroll to section three, press tab.. the element that receives focus is in section three. Previously, before implementing this, the element that received focus would be back in section one, which was kind of jarring.

[danielnixon]

Fair point. I still have a niggling fear that the experience won't be great for screen reader users, but this does indeed sound like a decent improvement for (sighted) keyboard users (at least in not-Safari).

This survey puts Safari usage among screen reader users at ~10%: https://webaim.org/projects/screenreadersurvey8/#browsers

[rafgraph]

Note that this is how the native browser experience works. In the original post for this issue:

Try opening this (non-SPA) link in your browser and then pressing tab key:

https://en.wikipedia.org/wiki/Firefox#Performance

(Focus will move to the first link after the #Performance ID)

Focus it not set on the #Performance element when clicking on the link (try it out in Chrome devtools, make sure Emulate focused page is enabled), but it is moved so pressing the tab key will focus the next element after #Performance.

My main goal for this library is to re-enable the native browser functionality that is lost when calling preventDefault() on the click event when using push state routing.

[maddy-jo]

thank you so much for this fix! looking forward to being able to upgrade and strip out some click handlers.

So the tradeoff is: 1) leave focus on the element that is scrolled to until a user event blurs it (as is the case in the above code), or 2) immediately blur the element after calling focus.

I wanted to ask about this element-blurring tradeoff as well. I see how the current approach is useful in terms of not imposing true focus behavior on a non-interactive element whose focusing steps wouldn't put focus on the element itself, but I also share some of @danielnixon's concern about losing focusing behavior where it is appropriate. for my use case, in line with the debate they linked at w3c/wcag#1001, I'd probably like to still be able to set visual focus styles on headings for accessibility reasons. and, doesn't apply to me right now, but conceivably you could have hash links to interactive elements, I guess.

I'm wondering if there's some kind of middle ground, like only calling element.blur() if the original element didn't have a tabindex and it's not an interactive element. that way, developers who wanted to leave focus on the link target could set tabindex="-1" (as I currently do for those elements in my workaround) and get that behavior without needing a special prop. regardless, thanks for your work on this; I know it's a tricky problem to recreate the native browser behavior that's being overridden, so I appreciate the thought you've put into this.

[rafgraph]

For anyone following this thread, @mjlumetta created a PR #73 so the focus behavior of react-router-hash-link matches the browser's native focus behavior. This is released in v2.3. For more info see the updated readme https://github.com/rafgraph/react-router-hash-link#focus-management