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

scrollTo() does not move focus to the element being scrolled to #24555

Closed
leafsy opened this issue Sep 16, 2019 · 3 comments
Closed

scrollTo() does not move focus to the element being scrolled to #24555

leafsy opened this issue Sep 16, 2019 · 3 comments

Comments

@leafsy
Copy link
Contributor

leafsy commented Sep 16, 2019

What's the issue?

Demo: https://codepen.io/leafsy/full/WNeypRE

In the example above, clicking on an anchor link would put focus on the element with corresponding id inside main content; however, clicking on an item with scrollTo action binding does not move focus to the element with corresponding id. For a11y best practice, the behaviors should be consistent.

What browsers are affected?

All browsers.

Which AMP version is affected?

Version 1909141411050

@dritter
Copy link
Contributor

dritter commented Sep 30, 2019

Your example was not valid AMP code.

This should work:

<!doctype html>
<html >
<head>
  <meta charset="utf-8">
  <title>responsive sidebar with autofocus</title>
  <link rel="canonical" href="amps.html">
  <meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
  <style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
  <script async custom-element="amp-sidebar" src="https://cdn.ampproject.org/v0/amp-sidebar-0.1.js"></script>
  <script async custom-element="amp-list" src="https://cdn.ampproject.org/v0/amp-list-0.1.js"></script>
  <script async custom-template="amp-mustache" src="https://cdn.ampproject.org/v0/amp-mustache-0.2.js"></script>
  <script async custom-element="amp-sidebar" src="https://cdn.ampproject.org/v0/amp-sidebar-0.1.js"></script>
  <script async src="https://cdn.ampproject.org/v0.js"></script>
  <style amp-custom>
    body {
      font-family: 'Courier New', Courier, monospace;
    }
    .hamburger{
      font-size: 2em;
      display:block;
      cursor: pointer;
      position: fixed;
      top: 0;
      right: 0;
      width: 40px;
      height: 40px;
      background: #03A9F4;
      z-index: 1;
    }
    h1, h2, h3, h4 {
      margin: 0px;
      padding: 5px;
      display: block;
    }
    header {
      background: #9C27B0;
      height: 100px;
    }
    article {
      background: #FFD54F
    }
    nav {
      margin: 0px;
    }
    aside {
      display: none;
    }
    ul.nav-container {
      display: block;
      padding: 0;
      background: #4CAF50;
      margin: 0px;
      width: 300px;
    }
    ul.nav-container > * {
      height: 100px;
      display: block;
      border-bottom: 1px solid #EEE;
      padding: 2px;
    }
    .selected {
      background: #FF9800;
    }
    /*  desktop */
    @media (min-width: 767px) {
      nav[toolbar] {
        position: sticky;
        top: 0;
        max-height: calc(100vh - 100px);
        overflow: auto;
        margin: 20px;
      }
      nav ul {
        display: block;
      }
      .hamburger {
        display: none;
      }
      main {
        display: flex;
        flex-direction: row;
      }
      aside {
        display: flex;
        flex-direction: column;
      }
      article {
        flex: 1
      }
    }
  </style>
</head>
<body>
  

  <amp-sidebar id="sidebar" layout="nodisplay">
      <nav toolbar="(min-width: 767px)" toolbar-target="desktop-sidebar">
     <ul>
       <li><span>anchor links</span></li>
       <li><a href="#a1">section 1</a></li>
       <li><a href="#a2">section 2</a></li>
       <li><a href="#a3">section 3</a></li>

       <li><span>scrollTo()</span></li>
       <li on="tap:sidebar.close(), a1.scrollTo(), a1.focus()" tabindex="0" role="nav">section 1</li>
       <li on="tap:sidebar.close(), a2.scrollTo(), a2.focus()" tabindex="0" role="nav">section 2</li>
       <li on="tap:sidebar.close(), a3.scrollTo(), a3.focus()" tabindex="0" role="nav">section 3</li>
      </ul>
    </nav>
  </amp-sidebar>

  <header>
      <h2>Header</h2>
      <button
        class="hamburger"
        on='tap:sidebar.toggle'
        aria-label="Click to open sidebar">
      =
    </button>
  </header>

   <main>
      <aside id="desktop-sidebar"></aside>

      <article>
        <h1>Main Content<h1>
        <h2 id="a1" tabindex="0">section 1</h2>
        <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
        <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
        <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
        <h2 id="a2" tabindex="0">section 2</h2>
        <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
        <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
        <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
        <h2 id="a3" tabindex="0">section 3</h2>
        <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
        <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
        <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
      </article>

  </main>
</body>
</html>

The main difference is that I had to define what should happen on click on the list item. I changed it to close the sidebar, scroll to the element and focus that element: on="tap:sidebar.close(), a1.scrollTo(), a1.focus()". This is indeed a bit verbose.

Other changes to your code:

  • The nav element in the amp-sidebar should only contain ul or ol elements
  • The correct syntax to use amp-mustace is to add it as custom-template (instead of custom-element)
  • Removed !important in CSS

@caroqliu caroqliu added this to Accessibility in wg-components Starter Issues Oct 12, 2020
@caroqliu caroqliu added this to Focus in A11y Week - Q4 2020 Oct 19, 2020
@dmanek
Copy link
Contributor

dmanek commented Oct 19, 2020

Seems like this issue is WAI. Will update the docs for scrollTo to pair with focus() as an a11y best practice.

@caroqliu
Copy link
Contributor

Closing as this is marked WAI and clarified in a follow up PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants