Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[google_maps_flutter_web] Options to disable tilt controls and configure gesture handling #4916

Closed
wants to merge 34 commits into from

Conversation

Rexios80
Copy link
Member

@Rexios80 Rexios80 commented Feb 24, 2022

tiltControlsEnabled: On web, in satellite view, and at close zoom levels, the map displays tilt controls. This lets you disable them.

gestureHandling: On web, without greedy gesture handling the map can have an overlay saying you can only move the map with two fingers or Cmd+drag. This is not ideal, and greedy gestures lets you make that not happen. I added the other gesture handling options from the Google Maps documentation for completeness.

List which issues are fixed by this PR. You must list at least one issue.

flutter/flutter/#99044

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@Rexios80
Copy link
Member Author

I wasn't sure how to depend on the updated google_maps_flutter_platform_interface without depending on it form the path. Is there some fancy way to do that? Or does it need to be published first?

@Rexios80
Copy link
Member Author

Oh they're supposed to be separate PRs. I guess I'll do that.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

@ditman Could you do the initial substantive review here, since this is primarily a web change?

@stuartmorgan-g
Copy link
Contributor

@ditman ping on this review from triage.

Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

This is a great addition!

I've done some comments wrt the current state of the maps JS SDK.

Also @stuartmorgan, there's a config value that might be similar across all platforms (that is named compassEnabled for native, which could serve as tiltControlsEnabled for the web.

Should we reuse the same boolean, documenting that "in web, it controls the rotationControls property?

@@ -72,9 +72,6 @@ gmaps.MapOptions _configurationAndStyleToGmapsOptions(

if (configuration.webGestureHandling != null) {
options.gestureHandling = configuration.webGestureHandling!.name;
} else if (configuration.scrollGesturesEnabled == false ||
Copy link
Member Author

Choose a reason for hiding this comment

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

@ditman Is this a breaking change? It doesn't break existing code, but it does change existing behavior.

@stuartmorgan-g
Copy link
Contributor

What's the status here? Is this waiting for another update, or for another round of review?

@Rexios80
Copy link
Member Author

This PR is waiting for another round of review

@stuartmorgan-g
Copy link
Contributor

@ditman Ping on this re-review (from triage).

@GaryQian
Copy link
Contributor

GaryQian commented Nov 1, 2022

@ditman Just a friendly reminder this is ready for another round of review :)

@ditman
Copy link
Member

ditman commented Nov 1, 2022

I'll get this reviewed (and maybe the branch updated) on Friday, November 4th. Apologies for the delay, I got busy with a couple of other super high priority issues :(

@stuartmorgan-g
Copy link
Contributor

@ditman Ping from triage; looks like this fell through the review cracks again.

@stuartmorgan-g
Copy link
Contributor

Update from triage: this is waiting for @ditman to have bandwith to review maps PRs.

@stuartmorgan-g
Copy link
Contributor

We've just completed the migration of the plugin code to the flutter/packages repository, as described in https://flutter.dev/go/flutter-plugins-repo-migration, and this repository is now being archived. Unfortunately that means that all in-progress PRs here must be moved to flutter/packages.

Please see our instructions for an explanation of how to move your PR, and if you have any issues moving your PR please don't hesitate to reach out in the #hackers-ecosystem channel in Discord.

Our apologies that your PR was caught in this one-time transition. We're aware that it's disruptive in the short term, and appreciate your help in getting us to a better long-term state!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants