Amphtml: amp-sidebar: optional attribute to change text for screen-reader close button.

[aghassemi]

Reserved For write/speak/code event

Description

In amp-sidebar, users can tap somewhere outside of the sidebar area to close it, however this is not accessible for blind users who rely on screen-readers to consume the Web. Therefore an alternative approach has been implemented that adds a visually hidden button to amp-sidebar which is discoverable by screen-readers to close the sidebar.

The Problem

Currently the text for this button is Close the sidebar and is not customizable so it can not be changed or translated.

We need to expose an optional attribute (data-close-button-aria-label) on amp-sidebar that, if set, will replace the text to the provided value.

The Fix

• Go to amp-sidebar.js around line 124. Note that the text for the screenReaderCloseButton is hard-coded to Close the sidebar
• Change the line above so the text becomes the value of the data-close-button-aria-label attribute if it is provided, it should still default back to Close the sidebar if data-close-button-aria-label is not provided.
• Update the amp-sidebar.md documentation file to include the new optional data-close-button-aria-label attribute.
• Update the test-amp-sidebar.js:
  
  
  
  • Update the existing should create an invisible close button for screen readers only test case to assert that the text of the button is Close the sidebar
  
  
  • Create a new test case by copying the test above, for this one we want to make sure the text changes when data-close-button-aria-label is present. To do that we need to:
    
    
    
    
    
    • Update getAmpSidebar() to optionally set that attribute ( e.g. if (options.closeText) { ampsetbar.setAttribute('data-close-button-aria-label', options.closeText) }; )
    
    
    
    • Use that option in this new test you created when calling getAmpSidebar() and then asserting he text is now something other than Close the sidebar.
    
    
    
  
  

Step by step

• [x] Claim this issue by adding a comment below. Please only claim this bug if you plan on starting work in the next day or so.
• [ ] If you aren't too familiar with Git/GitHub, see the Getting Started End-to-End Guide for an intro to Git & GitHub, and how to get a copy of the code. You can also refer to the Quick Start Guide for the necessary setup steps with less explanation than the End-to-End guide.
• [ ] Follow the instructions for building AMP.
• [ ] [Create a Git branch](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#create-a-git-branch) for making your changes.
• [ ] Do The fix as outlined above.
• [ ] [Commit your changes](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#edit-files-and-commit-them) frequently.
• [ ] [Push your changes to GitHub](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#push-your-changes-to-your-github-fork).
• [ ] [Create a Pull Request](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#send-a-pull-request-ie-request-a-code-review) and request a code review from me (@aghassemi). Mention Fixes #9867 in the description.
• [ ] [Respond to your reviewer's comments](https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-e2e.md#respond-to-pull-request-comments) (if any).

Once approved, your changes will be merged. ⚡⚡⚡Congrats on making your first contribution to the AMP Project!⚡⚡⚡ You'll be able to see it live across the web soon!

Thanks, and we hope to see more contributions from you soon.

Questions?

If you have questions ask in this issue or on your Pull Request (if you've created one) or see the How to get help section of the Getting Started guide.

[rhapsodyai]

I'll do this one

[mrjoro]

Hi @rhapsodyai; I sent you an invitation to join ampproject on GitHub; once you accept that I think we'll be able to assign this issue to you. Thanks!

[mrjoro]

Thanks for helping make AMP better @rhapsodyai!