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

[IOAPPX-403] Action in banner is not announced by screen reader on Android #342

Merged
merged 8 commits into from
Nov 6, 2024

Conversation

RiccardoMolinari95
Copy link
Collaborator

@RiccardoMolinari95 RiccardoMolinari95 commented Oct 24, 2024

Short description

This PR resolves an accessibility issue where the action in the Banner component was not properly announced by Screen Readers on Android devices. The changes introduced propose a solution while ensuring that existing functionality remains unaffected

List of changes proposed in this pull request

  • Introduced fallbackAccessibilityLabel, a dynamically generated label combining the title, content, and action if they are present.
  • If accessibilityLabel is not provided, fallbackAccessibilityLabel is used as a default

How to test

To test the changes, import the updated library containing the modifications into the IO app and ensure that the Screen Reader announces the title, content, and action accurately, even when one of these elements is missing.

Warning: This solution has been tested on an Android device and on an iOS simulator. Remains to be tested on iOS device

@RiccardoMolinari95 RiccardoMolinari95 requested review from dmnplb and a team as code owners October 24, 2024 15:30
Copy link
Collaborator

@dmnplb dmnplb left a comment

Choose a reason for hiding this comment

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

Hi @RiccardoMolinari95,
I read the description, but something doesn't click in my brain.
First of all, two things:

  • renderMainBlock is used in both the static and interactive versions, but from what I can see we're also editing the behaviour of the static one
  • Why can't we generate the complete accessibility label by checking if action is present?

Assuming I'm right, this is what I'm proposing:

  • Remove the actionAccessibility prop by dynamically generating the fallback accessibilityLabel based on the presence/absence of the action prop
  • Rename completeAccessibilityLabel to fallbackAccessibilityLabel
  • Change the statement in the renderMainBlock to accessibilityLabel ?? fallbackAccessibilityLabel, so that if we don't provide a valid accessibilityLabel, the component will use the generated fallback.

What do you think?

@RiccardoMolinari95
Copy link
Collaborator Author

Hi @RiccardoMolinari95, I read the description, but something doesn't click in my brain. First of all, two things:

  • renderMainBlock is used in both the static and interactive versions, but from what I can see we're also editing the behaviour of the static one
  • Why can't we generate the complete accessibility label by checking if action is present?

Assuming I'm right, this is what I'm proposing:

  • Remove the actionAccessibility prop by dynamically generating the fallback accessibilityLabel based on the presence/absence of the action prop
  • Rename completeAccessibilityLabel to fallbackAccessibilityLabel
  • Change the statement in the renderMainBlock to accessibilityLabel ?? fallbackAccessibilityLabel, so that if we don't provide a valid accessibilityLabel, the component will use the generated fallback.

What do you think?

Hi @dmnplb, yes you’re right! Removing actionAccessibility and using fallbackAccessibilityLabel does make the flow smoother and simplifies managing the accessibility label. Thanks

@dmnplb dmnplb merged commit 35c998e into main Nov 6, 2024
6 checks passed
@dmnplb dmnplb deleted the IOAPPX-403-action-banner-accessibility branch November 6, 2024 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants