Skip to content

Conversation

@vanessatran-ddi
Copy link
Collaborator

@vanessatran-ddi vanessatran-ddi commented May 14, 2025

Before (the change)

With a long content, when opening a modal, it scrolls into the middle

2410-before.mov

After (the change)

2410-after.mov

Make sure that you've checked the boxes below before you submit the PR

  • I have read and followed the setup steps
  • I have created necessary unit tests
  • I have tested the functionality in both React and Angular.

Steps needed to test

You can use the below angular code to test (similar vs React)

<goab-button (onClick)="openModal()">Show Modal</goab-button>
<goab-modal heading="Industry Description" [open]="open" [closable]="true" (onClose)="closeModal()">
  <h4>Business, building, and other support services</h4>
  <p>This industry is made up of two major sub industries:</p>
  <ol>
    <li>Management of companies and enterprises</li>
    <li>Administrative and support, waste management and remediation services</li>
  </ol>
  <h4>Management of companies and enterprises</h4>
  <p>This industry compprises:</p>
  <ol>
    <li>
      Legal entities engaged only in holding the securities of subsidiaries and
      affiliates for the purpose of owning a controlling interest or influencing the
      management decisions of these businesses, or
    </li>
    <li>
      Businesses primarily engaged in adminstering, overseeing, and managing other
      establishments of the company or enterprise and which may also hold the
      securities of their subsidiaries and affiliates.
    </li>
  </ol>
  <h4>Administrative and support, waste management and remediation services</h4>
  <p>This industry comprises of:</p>
  <ol>
    <li>
      Administrative and support services, estabilishments primarily engaged in
      activities such as administration, hiring, and placing personnel
    </li>
  </ol>
  <goab-button-group alignment="end" mt="l">
    <goab-button type="primary" (onClick)="closeModal()">
      I understand
    </goab-button>
  </goab-button-group>
</goab-modal>

and ts:

import { Component } from "@angular/core";
import { GoabButton, GoabButtonGroup, GoabModal } from "@abgov/angular-components";

@Component({
  selector: "abgov-issue-2410",
  standalone: true,
  templateUrl: "./issue-2410.component.html",
  imports: [GoabButtonGroup, GoabButton, GoabModal],
})
export class Issue2410Component {
  open = false;

  openModal() {
    this.open = true;
  }
  closeModal() {
    this.open = false;
  }
}

The below video is what I have tested with Windows NVDA screen reader. As expected, when open a modal, the title of the modal will be announced, when open an alertdialog modal, the full content will be announced, when Tab to move to the first focusable element, the first focusable element inside a modal is focused. When tab until the end of the modal and tab one more time, it goes back to the X (close button) and go to the next focusable element instead of exit the modal and focusing on somewhere else.

2410-modal-screen-reader.mov

@vanessatran-ddi vanessatran-ddi force-pushed the 2410-modal-accessibility branch from 83e2829 to 560b92c Compare May 14, 2025 22:54
@vanessatran-ddi
Copy link
Collaborator Author

@syedszeeshan The PR is ready for you to test. For this PR there are 2 files are "touched": FocusTrap and Modal. Beside testing the modal vs its accessibility (screen reader) and the bug 2410, you may need to test again to see if Drawer component which is using FocusTrap is working well.

@vanessatran-ddi vanessatran-ddi self-assigned this May 14, 2025
@vanessatran-ddi vanessatran-ddi linked an issue May 14, 2025 that may be closed by this pull request
@vanessatran-ddi vanessatran-ddi force-pushed the 2410-modal-accessibility branch from 560b92c to 7656601 Compare May 15, 2025 14:57
@syedszeeshan
Copy link
Collaborator

syedszeeshan commented May 15, 2025

image

Modal test PASSED....more testing in progress

@syedszeeshan
Copy link
Collaborator

syedszeeshan commented May 15, 2025

Drawer appears to be working fine also. PASSED.

@syedszeeshan
Copy link
Collaborator

syedszeeshan commented May 15, 2025

Screen Reader
When modal is opened, NVDA starts announcing/reading the modal content. PASSED.

@vanessatran-ddi
Copy link
Collaborator Author

Hi @chrisolsen and @ArakTaiRoth Please help me review this PR too. I have a 👍 from @syedszeeshan 😍

@ArakTaiRoth
Copy link
Collaborator

@chrisolsen This is ready for merge

@chrisolsen chrisolsen merged commit 940a899 into alpha May 27, 2025
4 checks passed
@chrisolsen chrisolsen deleted the 2410-modal-accessibility branch May 27, 2025 14:52
@tzuge
Copy link
Collaborator

tzuge commented May 27, 2025

🎉 This PR is included in version 1.34.1-alpha.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@tzuge tzuge added the released on @alpha Released into alpha. label May 27, 2025
@tzuge
Copy link
Collaborator

tzuge commented May 29, 2025

🎉 This PR is included in version 1.5.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@tzuge
Copy link
Collaborator

tzuge commented May 29, 2025

🎉 This PR is included in version 4.5.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@tzuge
Copy link
Collaborator

tzuge commented May 29, 2025

🎉 This PR is included in version 6.5.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@tzuge
Copy link
Collaborator

tzuge commented Jun 16, 2025

🎉 This PR is included in version 1.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@tzuge tzuge added the released Released into production. label Jun 16, 2025
@tzuge
Copy link
Collaborator

tzuge commented Jun 16, 2025

🎉 This PR is included in version 1.35.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@tzuge
Copy link
Collaborator

tzuge commented Jun 16, 2025

🎉 This PR is included in version 4.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@tzuge
Copy link
Collaborator

tzuge commented Jun 16, 2025

🎉 This PR is included in version 6.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

released on @alpha Released into alpha. released Released into production.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Modal: Opening in the middle, not at the top

7 participants