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 an iOS15 issue where Safari tab bar interrupts panning #11084

Merged
merged 4 commits into from
Oct 5, 2021

Conversation

mourner
Copy link
Member

@mourner mourner commented Oct 5, 2021

Tentatively closes #11048 (an issue on iOS where the iOS Safari tab bar interrupts panning):

  • Avoids calling stop on map.resize (originally introduced in gesture handling refactor #9365).
  • Makes resize do nothing if the container size didn't change (not necessary for the fix but felt like the right thing to do).

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • 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 an iOS15 issue where Safari tab bar interrupts panning</changelog>

@mourner mourner requested review from ansis and rreusser October 5, 2021 11:31
Copy link
Contributor

@ansis ansis left a comment

Choose a reason for hiding this comment

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

I think the intention of this.stop() was to stop gestures because they aren't written to handle resizing which could cause erratic jumps. However, I just tried gestures during a resize and they seem to work well enough with the new gesture implementation (because it's more pixel-based). I used https://github.com/mapbox/mapbox-gl-js/tree/test-gestures-while-resizing to test

The fix looks good to me. I didn't test on iOS 15.

The new unit test looks good, but it might be slightly better to add one that checks that the gesture still works with a resize. Maybe copy one of the touch tests in test/unit/ui/handler/drag_pan.test.js and throw a resize in there

@rreusser
Copy link
Contributor

rreusser commented Oct 5, 2021

The minimal page required to test this needs a map and enough content to scroll. This comment contains a sample page which can be dropped into debug/ for testing:

debug/issue-11048.html
<!DOCTYPE html>
<html>
<head>
    <title>Mapbox GL JS debug page</title>
    <meta charset='utf-8'>
    <meta name="viewport" content="width=device-width, initial-scale=1.0, user-scalable=no">
    <link rel='stylesheet' href='../dist/mapbox-gl.css' />
    <style>
        body { margin: 0; padding: 0; }
        html, body, #map { height: 100%; }
    </style>
</head>

<body>
<div style="min-height: 60vh"><div id="map" style="width: 300px; height: 300px"></div> <div>

Lucas ipsum dolor sit amet obi-wan skywalker solo jango jade grievous jango organa darth obi-wan. Tatooine gamorrean droid dantooine darth moff. Greedo leia darth maul jar r2-d2 jar secura hutt. Jabba mace cade palpatine. Ventress luke zabrak yoda lando organa mon hutt. Padmé skywalker calrissian moff. Antilles moff hoth moff jinn yavin maul. Hutt baba skywalker moff kenobi leia calrissian skywalker. Wookiee mothma binks boba ewok leia. Antilles palpatine jade jade jinn bespin sebulba. Kashyyyk yavin skywalker endor ackbar wedge calamari calamari cade.

Dagobah coruscant obi-wan moff ben kashyyyk antilles. C-3po yoda lars amidala mandalorians jabba antilles skywalker. Yoda dagobah amidala dantooine coruscant anakin. Utapau windu watto lando tatooine ben kenobi. Darth mandalore skywalker binks baba cade calrissian. Alderaan han maul anakin moff anakin. Vader naboo jinn moff aayla windu. Antilles secura ackbar hutt ackbar gonk. Organa moff jade skywalker r2-d2 wampa. Darth cade skywalker mandalore. Cade hutt maul calrissian. Tatooine jade organa yoda lando ackbar obi-wan coruscant obi-wan.

Dantooine solo owen coruscant moff windu organa moff. Wedge biggs coruscant hutt hutt lando mothma. Biggs solo lars secura dagobah. Jinn antilles tatooine jawa padmé skywalker antilles. Skywalker greedo jawa mon k-3po. Yoda mace lando c-3p0 sidious hutt mara chewbacca dagobah. Mon solo naboo yoda kamino wedge gonk. Ahsoka bothan calamari calrissian gamorrean. Baba kamino mace sith mon kit antilles. Jabba antilles yoda c-3po maul hutt solo. Antilles coruscant biggs darth skywalker. Boba skywalker wedge mustafar organa.

Mon mara luke fisto. Padmé grievous coruscant jango darth darth padmé hutt kessel. K-3po chewbacca k-3po moff cade tusken raider ahsoka palpatine. Moff vader moff jar bothan hutt. Obi-wan cade kenobi grievous skywalker hutt. Moff utapau kit darth owen wookiee kenobi jinn. Ben darth luuke ponda gamorrean hutt leia sebulba fett. Sidious organa twi'lek organa kessel. Tusken raider antilles boba jango dantooine. Binks antilles calrissian yavin mandalorians kessel ackbar. Hoth padmé antilles jango windu.

Fett yavin aayla lando amidala. Grievous darth palpatine darth. Skywalker skywalker biggs jawa windu antilles jabba moff mon. Dagobah moff windu bothan. Skywalker moff padmé fisto jango darth ventress mothma. Organa moff moff yavin alderaan. Skywalker calrissian skywalker mustafar skywalker twi'lek. Jawa sith calamari solo moff moff ackbar. Ben endor kessel aayla. Solo lando thrawn amidala dooku qui-gonn binks. Skywalker hutt yoda dooku obi-wan sidious grievous coruscant jango. Skywalker aayla moff solo ewok anakin yoda dagobah dooku.

<script src='../dist/mapbox-gl-dev.js'></script>
<script src='../debug/access_token_generated.js'></script>
<script>

var map = window.map = new mapboxgl.Map({
    container: 'map',
    zoom: 12.5,
    center: [-122.4194, 37.7749],
    style: 'mapbox://styles/mapbox/streets-v11',
    hash: true
});

</script>

</html>

@rreusser
Copy link
Contributor

rreusser commented Oct 5, 2021

Here's what it looks like when I rotate the device while panning. Since the map size changes, it jumps a little bit. We could consider transforming coordinates and smoothing this out, but I don't think it's particularly awful since this interaction pattern is probably exceptionally uncommon.

I'm definitely okay with this behavior.

IMG_0101.mp4

Copy link
Contributor

@rreusser rreusser left a comment

Choose a reason for hiding this comment

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

I'm going to look at adding the additional test mentioned by @ansis, but this looks perfect to me. I tested it on iOS 12 and iOS 15, and it works as expected. Thanks, @mourner!! 🙇

@rreusser
Copy link
Contributor

rreusser commented Oct 5, 2021

@ansis does this test match what you had in mind? d82b00f

@rreusser
Copy link
Contributor

rreusser commented Oct 5, 2021

There's one more situation which causes this bug: when you first navigate to the page and it toggles the tab bar, a blur event is emitted which triggers this line, which interrupts interactions:

this.stop(true);

Contemplating if we want to disable or modify this behavior. 🤔

@rreusser
Copy link
Contributor

rreusser commented Oct 5, 2021

Disabling halt-on-window-blur has the following undesirable effect on desktop, in which interactions continue in a background window as long as you hold down the mouse button:

blur

Copy link
Contributor

@SnailBones SnailBones left a comment

Choose a reason for hiding this comment

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

Tested this on Android Firefox, and it fixes a similar but unreported bug that likely occurs in other browsers that show/hide a search bar on swipe. (In Firefox, the issue occurs only when tilting with a two-finger swipe.)

I'm not seeing any issues in Android Chrome.

Great work @mourner and all who are tackling this!

* Move blur event reset into non-touch handlers

* Fix linter

* Fix/amend unit tests

* Flush task queue in rotate test
@rreusser rreusser merged commit 23ff998 into main Oct 5, 2021
@rreusser rreusser deleted the mourner/fix-ios-15 branch October 5, 2021 21:56
rreusser added a commit that referenced this pull request Oct 5, 2021
* fix an iOS15 issue where map stops when panning

* fix tests and lint

* Test drag pan handler does not end interaction on resize

* Move blur event reset into non-touch handlers (#11087)

* Move blur event reset into non-touch handlers

* Fix linter

* Fix/amend unit tests

* Flush task queue in rotate test

Co-authored-by: Ricky Reusser <ricky.reusser@mapbox.com>
Co-authored-by: Ricky Reusser <rreusser@users.noreply.github.com>
rreusser added a commit that referenced this pull request Oct 6, 2021
…11089)

* fix an iOS15 issue where map stops when panning

* fix tests and lint

* Test drag pan handler does not end interaction on resize

* Move blur event reset into non-touch handlers (#11087)

* Move blur event reset into non-touch handlers

* Fix linter

* Fix/amend unit tests

* Flush task queue in rotate test

Co-authored-by: Ricky Reusser <ricky.reusser@mapbox.com>
Co-authored-by: Ricky Reusser <rreusser@users.noreply.github.com>

Co-authored-by: Vladimir Agafonkin <agafonkin@gmail.com>
avpeery pushed a commit that referenced this pull request Oct 6, 2021
…11089)

* fix an iOS15 issue where map stops when panning

* fix tests and lint

* Test drag pan handler does not end interaction on resize

* Move blur event reset into non-touch handlers (#11087)

* Move blur event reset into non-touch handlers

* Fix linter

* Fix/amend unit tests

* Flush task queue in rotate test

Co-authored-by: Ricky Reusser <ricky.reusser@mapbox.com>
Co-authored-by: Ricky Reusser <rreusser@users.noreply.github.com>

Co-authored-by: Vladimir Agafonkin <agafonkin@gmail.com>
avpeery added a commit that referenced this pull request Oct 7, 2021
…11101)

* Fix an iOS15 issue where Safari tab bar interrupts panning (#11084) (#11089)

* fix an iOS15 issue where map stops when panning

* fix tests and lint

* Test drag pan handler does not end interaction on resize

* Move blur event reset into non-touch handlers (#11087)

* Move blur event reset into non-touch handlers

* Fix linter

* Fix/amend unit tests

* Flush task queue in rotate test

Co-authored-by: Ricky Reusser <ricky.reusser@mapbox.com>
Co-authored-by: Ricky Reusser <rreusser@users.noreply.github.com>

Co-authored-by: Vladimir Agafonkin <agafonkin@gmail.com>

* Removed getBoundingClientRect conflict for unit tests

* Added offsetHeight to replace getBoundingClientRect to fix unit tests

* add clientWidth and clientHeight to unit tests

* container -> map.getContainer()

* Replaced offsetWidth with clientWidth

* removing change difference in attribution and logo unit tests from v1.13.2

* Change size of container instead of canvas container to trigger resize in unit tests

* Added change in height to trigger resize

* fix to attribution.test.js

* Fixes logo.test.js to pass

* removed unneeded changes

Co-authored-by: Ricky Reusser <rreusser@users.noreply.github.com>
Co-authored-by: Vladimir Agafonkin <agafonkin@gmail.com>
katydecorah pushed a commit that referenced this pull request Oct 20, 2021
* main:
  Add touch pan blocker to gesture handling for touch devices (#11116)
  Address accessibility issues (#11064)
  add support for non-mercator projections (#11124)
  Image fallback expressions within paint properties (#11049)
  Replaces EPSG:4326 with OGC:CRS84 in GL JS `LngLat` doc (#11072)
  Add globe view support to heatmap shaders (#11120)
  Exclude flaky test (#11118)
  consistify YOUR_MAPBOX_ACCESS_TOKEN as placeholder string (#11113)
  Allow adding multiple layers to `map.on()` event handler (h/t @omerbn) (#11114)
  render-test-flakiness:clear worker storage (#11111)
  upgrade to supercluster v7.1.4, earcut v2.2.3, vt-pbf v3.1.3, geojson-rewind v0.5.1 (#11110)
  Added v1.13.2 changelog entry (#11108)
  One weird JSON.parse() trick (#11098)
  Fixed doc usage of map.getCenter (#11093)
  s̶y̶m̶b̶o̶l̶-̶c̶l̶i̶p̶ dynamic-filtering with `pitch` and `distance-from-camera` expressions (#10795)
  Update link to transpiling guide (#11096)
  Cherry pick 2.5.1 changelog (#11099)
  Fix an iOS15 issue where Safari tab bar interrupts panning (#11084)
  Fix conditional check for isFullscreen to accommodate Safari (#11086)
  Render tests for #11041 (#11070)
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.

iOS 15 Safari tab bar interrupts map interactions
4 participants