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) #11101

Merged
merged 12 commits into from
Oct 7, 2021

Conversation

avpeery
Copy link
Contributor

@avpeery avpeery commented Oct 6, 2021

Cherry-picks #11089 (including, by extension, #11087) into release v1.13.2.

  • 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 (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

…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>
@rreusser
Copy link
Contributor

rreusser commented Oct 6, 2021

Ah, I think the failing test is because I wrote a test that followed the getBoundingClientRect approach to issue a resize. I think you'll need to look at what the syntax was before that change, e.g.:

https://github.com/mapbox/mapbox-gl-js/pull/10936/files#diff-769705513300ce3f3807890b1b8dd7498fdb7e71d428a81d3f27941a70bf01d4L750-L751

(It would definitely be best to fix, but just in case that's really not straightforward for some reason, I wouldn't be extremely opposed to dropping that particular test since changes to these old release branches are so rare.)

Copy link
Contributor

@arindam1993 arindam1993 left a comment

Choose a reason for hiding this comment

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

LGTM! thanks for tackling this!

@avpeery
Copy link
Contributor Author

avpeery commented Oct 6, 2021

Still having two unit tests fail: 'AttributionControl appears in compact mode if container is less then 640 pixel wide' and 'LogoControl appears in compact mode if container is less then 250 pixel wide'... I switched to clientWidth thinking possibly offWidth was causing it to fail. These two tests were here previously. Any ideas @rreusser ?

@avpeery
Copy link
Contributor Author

avpeery commented Oct 6, 2021

I'm removing any change differences to those two unit tests from v1.13.2 branch, if it still breaks I think this line is causing it to break.... I suspect it's because it was changing the canvas container size in the unit tests instead of the container size, so it didn't trigger the resize. @SnailBones @arindam1993 @rreusser Would there ever be a case where there was a resize of the canvas and not container in the previous versions? Maybe there is a fix in a later version that resizes the container if the canvas was resized...

@arindam1993
Copy link
Contributor

@avpeery Is the compact mode Attribution and logo working in a debug page, tho?

@avpeery
Copy link
Contributor Author

avpeery commented Oct 7, 2021

@arindam1993 - Yes both work in the debug page.

@avpeery
Copy link
Contributor Author

avpeery commented Oct 7, 2021

@arindam1993 @rreusser @SnailBones attribution and logo compact mode both work in debug pages. It seems the two tests are failing at the container is a fake object which is does not have transform.width or transform.height. Would it be better to remove these two unit tests or attempt to resolve this?

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.

Once it looks good to circle-ci, LGTM!

@avpeery avpeery merged commit afc4556 into release-v1.13.2 Oct 7, 2021
@avpeery avpeery deleted the avpeery/1.13.2-cherry-pick branch October 7, 2021 18:36
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.

None yet

4 participants