amp-sideba

[loadingscreen78]

Description

The Issue: amp-sidebar Accessibility Bug ♿ Problem: The button that opens and closes the component does not correctly update its aria-expanded attribute. This attribute is essential for screen readers, as it tells users with visual impairments whether a collapsible element (like a sidebar menu) is currently open or closed.

Expected Behavior:

When the sidebar is closed, the button controlling it should have the attribute aria-expanded="false".

When the sidebar is opened, the button's attribute should change to aria-expanded="true".

Currently, this attribute is not being updated, which can confuse users who rely on assistive technology.

Why This is a Good First Issue ✅ High Impact: You'll be making the web more accessible for everyone.

Isolated Scope: The fix is contained within the component's logic. You won't need to understand the entire AMP codebase to solve it.

Clear Goal: It's easy to test and verify if your fix works. Either the attribute updates, or it doesn't.

Great Introduction: This will introduce you to handling user actions, manipulating the DOM, and writing tests within a major open-source project.

Reproduction Steps

How to Get Started 🚀 Find the Issue on GitHub: Go to the ampproject/amphtml GitHub issues page. Filter the issues by the label good first issue to find this one or other similar tasks.

Claim the Issue: Leave a comment on the issue thread saying you'd like to work on it. A project maintainer will usually assign it to you.

Set Up Your Environment: Follow the project's CONTRIBUTING.md file. This guide has all the instructions for forking the repository, installing dependencies, and running the project locally.

Implement the Fix: Locate the JavaScript file for the amp-sidebar component. Find the code that handles the open/close toggle and add the logic to set the aria-expanded attribute on the button.

Submit a Pull Request (PR): Once you've tested your changes, submit a PR. The project maintainers will review your code, provide feedback, and merge it when it's ready.

Relevant Logs

Relevant Logs
For this client-side UI and accessibility issue, there are no errors logged to the browser's Console. The relevant evidence is found by inspecting the DOM and the Accessibility Tree using browser developer tools.

Observations from Chrome DevTools (Elements & Accessibility Panel):

Initial State (Sidebar Closed):

The button controlling the sidebar is inspected.

The aria-expanded attribute is correctly set to "false".

Log:

HTML

<button on="tap:my-sidebar.toggle" aria-expanded="false">
  Open Menu
</button>
Action (User clicks to open sidebar):

The sidebar correctly opens and becomes visible.

The aria-expanded attribute on the button fails to update. It remains "false".

Log:

HTML

<button on="tap:my-sidebar.toggle" aria-expanded="false">
  Open Menu
</button>
The Accessibility Inspector confirms that screen readers are being informed that the element is still collapsed, which is incorrect.

Action (User clicks to close sidebar):

The sidebar correctly closes.

The aria-expanded attribute remains "false".

Summary: The browser console is clean. The bug is a silent failure to update a DOM attribute, confirmed by live inspection of the element's properties during interaction.












Tools

Browser(s) Affected

No response

OS(s) Affected

No response

Device(s) Affected

No response

AMP Version Affected

No response

[Moresh-Mishra]

Hello dev. Please assign this to me

[loadingscreen78]

i am really sorry it seems i donot have the permissions to make this type of call .

[Moresh-Mishra]

No problem, will wait for someone to assign

[Siddu-06-0405]

I’d like to work on this. Could you assign me the issue?

[loadingscreen78]

i am really sorry it seems i donot have the permissions to make this type of call .

[Siddu-06-0405]

No worries, thanks for clarifying! I’ll wait for the maintainers to assign.

[Siddu-06-0405]

Hey @loadingscreen78 @Moresh-Mishra 👋

I just wanted to give an honest update: I tried my absolute best to tackle this bug… and I have to admit that I failed pretty spectacularly 😅

This was my first ever “good first issue,” and I went in full of enthusiasm. I set up the repo, poked around amp-sidebar, added logs everywhere, stared at DevTools for hours, and desperately tried different spots to update the aria-expanded attribute.

But no matter what I did, the attribute refused to update. It just sat there stubbornly on "false", mocking me every time I clicked the button. At one point I was convinced I was fighting an invisible boss battle inside the AMP codebase.

import { isExperimentOn } from '#experiments';

import { AmpPreactBaseElement, setSuperClass } from '#preact/amp-base-element';

import { Services } from '#service/';

import { userAssert } from '#utils/log';

import { BaseElement } from './base-element';

import { CSS } from '../../../build/amp-sidebar-1.0.css';

/** @const {string} */
const TAG = 'amp-sidebar';

class AmpSidebar extends AMP.BaseElement {
  /** @override */
  constructor(element) {
    console.log("AmpSidebar class loaded");
    super(element);

    /** @private {!../../../src/service/history-impl.History} */
    this.history_ = null;

    /** @private {number|null} */
    this.historyId_ = null;
  }

  /** @override */
  init() {
    console.log("AmpSidebar class loaded")
    this.history_ = Services.historyForDoc(this.getAmpDoc());

    this.registerApiAction('toggle', (api) => api./*OK*/ toggle());
    this.registerApiAction('open', (api) => api./*OK*/ open());
    this.registerApiAction('close', (api) => api./*OK*/ close());

    return super.init();
  }

  /** @override */
  attachedCallback() {
    super.attachedCallback();
    if (
      this.element.parentNode != this.element.ownerDocument.body &&
      this.element.parentNode != this.getAmpDoc().getBody()
    ) {
      this.user().warn(
        TAG,
        `${TAG} is recommended to be a direct child of the <body>` +
        ` element to preserve a logical DOM order.`
      );
    }
  }

  /** @override */
  isLayoutSupported(unusedLayout) {
    userAssert(
      isExperimentOn(this.win, 'bento') ||
      isExperimentOn(this.win, 'bento-sidebar'),
      'expected global "bento" or specific "bento-sidbar" experiment to be enabled'
    );
    return true;
  }

  /** @override */
  afterOpen() {
    super.afterOpen();
    const sidebar = this.element.shadowRoot.querySelector('[part=sidebar]');
    this.setAsContainer?.(sidebar);

    console.log("hello");
    // Update aria-expanded 
    const toggleButton = this.element.ownerDocument.querySelector(`[on = "tap:${this.element.id}.toggle"]`);
    if (toggleButton) { 
      toggleButton.setAttribute('aria-expanded', 'true'); 
    }

    this.history_
      .push(() => this.api().close())
      .then((historyId) => (this.historyId_ = historyId));
  }

  /** @override */
  afterClose() {
    super.afterClose();
    this.removeAsContainer?.();

    if (this.historyId_ != null) {
      this.history_.pop(this.historyId_);
      this.historyId_ = null;
    }
  }

  /** @override */
  unmountCallback() {
    this.removeAsContainer?.();
  }
}

AMP.extension(TAG, '1.0', (AMP) => {
  AMP.registerElement(TAG, AmpSidebar, CSS);
});

<!doctype html>
<html ⚡ lang="en">
<head>
  <meta charset="utf-8">
  <link rel="canonical" href="./regular-html-version.html">
  <meta name="viewport" content="width=device-width,minimum-scale=1">
  <script async src="https://cdn.ampproject.org/v0.js"></script>
  <script async custom-element="amp-sidebar" src="http://127.0.0.1:8000/dist/v0/amp-sidebar-1.0.js"></script>
  <style amp-custom>
    body { font-family: sans-serif; padding: 20px; }
    button { font-size: 16px; padding: 8px 12px; }
    amp-sidebar { background: #eee; width: 250px; }
    amp-sidebar ul { list-style: none; padding: 0; margin: 0; }
    amp-sidebar li { padding: 10px; border-bottom: 1px solid #ccc; }
  </style>
</head>
<body>

  <h1>AMP Sidebar Example</h1>
  <!-- Toggle button -->
  <button on="tap:sidebar1.toggle">Open Sidebar</button>

  <!-- Single Sidebar -->
  <amp-sidebar id="sidebar1" layout="nodisplay" side="left">
    <ul>
      <li><a href="#">Nav item 1</a></li>
      <li><a href="#">Nav item 2</a></li>
      <li><a href="#">Nav item 3</a></li>
    </ul>
  </amp-sidebar>

</body>
</html>

what am i doing wrong here?

[loadingscreen78]

Thanks for your hard work appreciate it bruv

[panagiotisbellias]

I'd like to work on it. Can a project maintainer assign it to me? Moreover, I can't find CONTRIBUTING.md file and local setup instructions, can you help?

[mahir21]

Hi Guys I am new to open source contribution. I have forked the repository and cloned the project on my local machine. However one challenge I am facing is how do I understand which file the issue is in. There is so many files. For example there is few amp html files. How do I located which amp html file this is taking about.

[panagiotisbellias]

Hi! Just checking if there’s any update on this. I’d be happy to work on it and can prepare a PR once I have the guidance needed.