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

Maps: Does not support overriding accessibility #305

Closed
jacob-williams-kr opened this issue Apr 11, 2023 · 15 comments · Fixed by #407
Closed

Maps: Does not support overriding accessibility #305

jacob-williams-kr opened this issue Apr 11, 2023 · 15 comments · Fixed by #407
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. released type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@jacob-williams-kr
Copy link

Environment details

Compose Maps: com.google.maps.android:maps-compose:2.11.4

Steps to reproduce

The Compose GoogleMap function does not support overriding the accessibility options. In my case, I am trying to get the map to not be read out by screen readers. Previously, with the XML setup, we did this with android:importantForAccessibility="noHideDescendants", however using clearAndSetSemantics does not work for the compose view.

Code example

GoogleMap(
        cameraPositionState = cameraPositionState,
        modifier = Modifier.clearAndSetSemantics {},
) 

Markers also do not support a custom modifier, meaning they cannot have custom screen readouts, or custom tagging for tests

@jacob-williams-kr jacob-williams-kr added triage me I really want to be triaged. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Apr 11, 2023
@wangela
Copy link
Member

wangela commented Apr 11, 2023

If you would like to upvote the priority of this issue, please comment below or react with 👍 so we can see what is popular when we triage.

@jacob-williams-kr Thank you for opening this issue. 🙏
Please check out these other resources that might help you get to a resolution in the meantime:

This is an automated message, feel free to ignore.

@lucas-livefront
Copy link

lucas-livefront commented May 17, 2023

This is significantly affecting me. I am making new features for an app with millions of users. I want to disable all accessibility features for the map and its pins. We have specific features for accessibility users that we want to guide them to but this bug limits our ability to do so..

@lucas-livefront
Copy link

@wangela & @jacob-williams-kr I apologize for @ing you but I am getting a little desperate. I will need to make a talk back work around for this issue so I just want to check in for an update here before I start some complex solution.

I did make this SO post as well to no avail.

@kikoso kikoso added priority: p3 Desirable enhancement or fix. May not be included in next release. and removed triage me I really want to be triaged. labels Jul 10, 2023
@AdamNagy-nagya
Copy link

This issue significantly impacts our project's efforts to enhance accessibility support.

We cannot modify or remove semantic properties using modifiers.

Moreover, TalkBack consistently announces "Double tap to activate," regardless of whether we include any onClick callbacks. This behavior remains even when we're exclusively utilizing it as a non-interactive static map, without any enabled gestures or controls.

It's worth noting that tapping directly on a marker also triggers focus when using TalkBack. It will be announced as "Marker title Double tap to activate."

To mitigate this, the only solution we've found for the map is as follows:

Box(
    modifier = modifier
        .semantics(mergeDescendants = true) { }
        .clearAndSetSemantics { contentDescription = "Google Map" }
) {
    GoogleMap(
         ... 

This approach ensures that the map itself is not announced as an interactive element. However, this workaround cannot be applied to markers.

@mightymia
Copy link

We have a large app that services millions of people worldwide every day; this is a significant issue for our travelers using talkback. I want the option to remove the click on the map and hide the marker from talkback because, in our use case, it is for decoration purposes only.

@wangela
Copy link
Member

wangela commented Sep 19, 2023

Thank you for sharing how this can help wtih accessibility for apps built with the Maps Compose library. Raising the priority and will evaluate whether we can raise it even further.

@wangela wangela added priority: p2 Moderately-important priority. Fix may not be included in next release. and removed priority: p3 Desirable enhancement or fix. May not be included in next release. labels Sep 19, 2023
@kikoso kikoso self-assigned this Sep 20, 2023
@kikoso
Copy link
Collaborator

kikoso commented Sep 20, 2023

Hi folks. I have found a way to wrap-up the Google Map in a box (similar to the solution by @AdamNagy-nagya), and pass the Modifiers as parameters.

However, I am not sure if this can be applied individually to markers, since in the XML view-based system we can't add any XML tag for them.

What would be the ask here for markers?

@wangela
Copy link
Member

wangela commented Oct 7, 2023

Inviting the folks here who have mentioned markers to review PR #407 and request modifications to provide the necessary accessibility support for markers. @jacob-williams-kr @AdamNagy-nagya @mightymia

@kikoso
Copy link
Collaborator

kikoso commented Nov 1, 2023

Hi folks.

The PR has been updated with a solution that should solve the reported problems.

First: android:importantForAccessibility="noHideDescendants" does not work for obvious reasons on Compose. The alternative, passing a modifier with clearSemantics, does not work for two reasons:

1.- The GoogleMap implementation takes over consistent ownership of updating the semantics of MapView when this is rendered. This happens in the MapUpdater.

2.- Then, of course, android-maps-compose is an abstraction library at the top of GoogleMaps, so even if we could pass clearSemantics, what does this mean? Ultimately, this could be equivalent to android:importantForAccessibility="noHideDescendants"

The current PR provides a couple of changes in the implementation:

1.- GoogleMaps has now a mergeDescendants parameter, that resembles what Compose would do. If this parameter is set to true, then the MapUpdater performs the equivalent XML function:

if (mergeDescendants) {
        mapView.importantForAccessibility = IMPORTANT_FOR_ACCESSIBILITY_NO_HIDE_DESCENDANTS
    }

This allows us to keep a more or less Compose friendly API, and achieve the results set via XML.

2.- Because GoogleMap implementation takes over consistent ownership of updating the semantics of MapView, we need to set the new semantic with contentDescription.

3.- Markers contain now a new contentDescription attribute that supersedes the title for accessibility purposes.

There is a new example file showcasing how this works. This PR does not break the previously exposed APIs.

@jacob-williams-kr , @AdamNagy-nagya , @mightymia , let us know if you have any questions!

@mightymia
Copy link

This is great, thank you! When can we expect this to be available to use?

@kikoso
Copy link
Collaborator

kikoso commented Nov 2, 2023

Rather soon, @mightymia ! I will discuss this latest by tomorrow with @wangela .

@googlemaps-bot
Copy link
Contributor

🎉 This issue has been resolved in version 4.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@kikoso
Copy link
Collaborator

kikoso commented Nov 13, 2023

@mightymia , this has been merged and it is included in 4.2.0. Let us know if you have any further questions.

@mightymia
Copy link

Thank you @kikoso, I've made a note in our ticket to update to the 4.2.0 version for the fix.

@lucas-livefront
Copy link

Bravo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. released type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants