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

fix(popover-container): ensure component is accessible by screen readers #7153

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

nuria1110
Copy link
Contributor

fix #7133

Proposed behaviour

  • The popover dialog gains focus when opened to allow screen readers to access the popup.
  • Adds hasFullWidth prop to enable fullWidth Button behaviour optionally.
  • Expose focusButton method to enable consumers to programatically focus the open button.
  • Ensure Form within a PopoverContainer had expected overflow styling and stickyFooter has correct border-radius.

image

Current behaviour

  • The component renders in a Portal and cannot be accessed by screen reader commands.
  • Component has 100% width causing other elements to render below it.
  • Form within a PopoverContainer does not have overflow styling.

image

Checklist

  • Commits follow our style guide
  • Related issues linked in commit messages if required
  • Screenshots are included in the PR if useful
  • All themes are supported if required
  • Unit tests added or updated if required
  • Playwright automation tests added or updated if required
  • Storybook added or updated if required
  • Translations added or updated (including creating or amending translation keys table in storybook) if required
  • Typescript d.ts file added or updated if required
  • Related docs have been updated if required

QA

  • Tested in provided StackBlitz sandbox/Storybook
  • Add new Playwright test coverage if required
  • Carbon implementation matches Design System/designs
  • UI Tests GitHub check reviewed if required

Additional context

Testing instructions

Adds hasFullWidth flag to enable fullWidth Button behaviour and remove the default 100% width on the
component.
…yling

Fixes styling issues on a Form with stickyFooter within a PopoverContainer, adds border-radius and
overflow styling so that form content can render as a scrollable container. Also prevents tabbing
onto the form element.
…ally

Exposes `focusButton` method to allow consumers to programmatically focus on the open button using
ref.
@@ -86,7 +86,7 @@ You can easily use many different components to create your own composition.
### Filter component

If you want to use the `PopoverContainer` to create for example `Filter` component.
You can do it easly in this way:
You can do it easily in this way:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise(non-blocking): Well spotted.

@@ -391,15 +389,15 @@ describe("opening the popup", () => {
},
);

it("open button still has focus after popup is opened", async () => {
it("focus moves to dialog when popup is opened", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick/suggestion: Just changing a word around to make the test description make grammatical sense.

Suggested change
it("focus moves to dialog when popup is opened", async () => {
it("moves focus to dialog when popup is opened", async () => {

Copy link
Contributor

@damienrobson-sage damienrobson-sage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

The component renders the popup within a Portal due to potential cascading styles when placed within
a MenuItem. To ensure the popup is accessible by screen readers, the dialog will now gain focus when
opened.

fix #7133
@nuria1110 nuria1110 marked this pull request as ready for review January 15, 2025 15:28
@nuria1110 nuria1110 requested review from a team as code owners January 15, 2025 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

[Popover Container] VoiceOver issue when popover-container-content is opening at the end of node tree.
3 participants