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

Disable cooperative gestures in fullscreen for touch panning and add screen reader alert for cooperative gestures warning message #12058

Merged
merged 4 commits into from
Jul 6, 2022

Conversation

SnailBones
Copy link
Contributor

@SnailBones SnailBones commented Jul 1, 2022

Fixes #11635
Fixes https://github.com/mapbox/gl-js-team/issues/458

Before:

voiceover-main.mov

After:

voiceover-fix.mov

Launch Checklist

  • briefly describe the changes in this PR
  • include before/after visuals or gifs if this PR includes visual changes
  • write tests for fullscreen fix
  • Add accessibility to unit tests
  • manually test the debug page
  • apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
  • add an entry inside this element for inclusion in the mapbox-gl-js changelog: <changelog>Fix cooperativeGestures preventing panning on mobile while in fullscreen. Add screen reader alert for cooperative gestures warning message.</changelog>

@SnailBones SnailBones changed the title Disable cooperative gestures in fullscreen for touch panning Disable cooperative gestures in fullscreen for touch panning and adding screen reader support for cooperative gestures warning message Jul 5, 2022
@SnailBones SnailBones changed the title Disable cooperative gestures in fullscreen for touch panning and adding screen reader support for cooperative gestures warning message Disable cooperative gestures in fullscreen for touch panning and add screen reader alert for cooperative gestures warning message Jul 5, 2022
Copy link
Contributor

@karimnaaji karimnaaji left a comment

Choose a reason for hiding this comment

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

LGTM, few questions but nothing blocking.

src/util/util.js Show resolved Hide resolved
@@ -112,7 +112,7 @@ class ScrollZoomHandler {
* const isScrollZoomEnabled = map.scrollZoom.isEnabled();
*/
isEnabled(): boolean {
return !!this._enabled;
return this._enabled;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this diff, as this._active, this._zooming and this._enabled may not be initialized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point Karim! It seemed a bit strange to be applying !! to a value that's already a boolean, but I now realize that this is potentially a breaking change. Thoughts on setting these values to false in the constructor?

Copy link
Contributor

Choose a reason for hiding this comment

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

false in the constructor works, but also not against leaving as is!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Readded the !!s, I've also added them to TouchPanHandler for consistency.

@karimnaaji karimnaaji merged commit ac7f8e8 into main Jul 6, 2022
@karimnaaji karimnaaji deleted the aidan/touch-pan-fullscreen branch July 6, 2022 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cooperative gestures persists in fullscreen on Android
2 participants