Skip to content

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

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

Merged
merged 41 commits into from
Jul 24, 2023

Conversation

Rexios80
Copy link
Member

Transferred from flutter/plugins#4916

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.

@reidbaker
Copy link
Contributor

Thank you for your contribution. It looks like the tests are failing. Before we review the PR, please see what you can do to resolve the test failures. If you are unsure how to proceed, please reach out for help on the #hackers-new channel.

@Rexios80
Copy link
Member Author

Rexios80 commented Mar 3, 2023

@reidbaker The actions seem to only be failing due to the required dependency overrides since this PR requires changes to the platform interface. Until we are ready to split out the platform interface changes into their own PR and get that merged, the actions will continue to fail on this PR.

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.

Minor comments, but generally this looks good to be at an API level. I'll defer to @ditman on whether we have any better options in terms of how to expose this, but it seemed from the discussion in the previous iteration like we probably can't abstract it.

@Rexios80
Copy link
Member Author

Rexios80 commented Mar 7, 2023

I think these checks are as good as they're getting as long as we have dependency overrides

@reidbaker reidbaker removed their request for review March 8, 2023 17:58
@stuartmorgan-g
Copy link
Contributor

Update from triage: This is just waiting for @ditman to have cycles to review it.

@cyanglaz cyanglaz removed their request for review May 8, 2023 19:43
@stuartmorgan-g
Copy link
Contributor

Update from triage: This is still waiting for @ditman to have bandwidth to review maps PRs.

@Rexios80
Copy link
Member Author

@ditman @stuartmorgan #4521

auto-submit bot pushed a commit that referenced this pull request Jul 21, 2023
@Rexios80 Rexios80 requested a review from ditman July 21, 2023 16:27
@Rexios80
Copy link
Member Author

@ditman @stuartmorgan Can we merge this now?

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.

LGTM

This just needs a quick final stamp from @ditman to land.

@ditman
Copy link
Member

ditman commented Jul 24, 2023

I hope that in the future we won't need any more special web-only config parameters (or that they'll map better to their mobile counterparts if only because of the rendering engines are more similar!)

Here's some more web-only parameters incoming: #4553

Before we merge the web implementation of this, let me steer 4553 to create a new "webOptions" object or similar, where we add the 3 options proposed in 4553, and add the 45deg imagery and gesture handling from here, then we'll update the web implementation to read from the new "webOptions" object, instead of directly from the MapConfiguration.

/cc @stuartmorgan

@ditman
Copy link
Member

ditman commented Jul 24, 2023

(Discussed on chat with @stuartmorgan and we decided to go with the PRs as they are, and then maybe do a more thorough refactor of per-platform settings, for example "liteMode" doesn't make sense in the web or iOS either)

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.

LGTM, let's go!

@ditman ditman added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 24, 2023
@auto-submit auto-submit bot merged commit 18d5506 into flutter:main Jul 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 25, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 25, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 25, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jul 25, 2023
flutter/packages@8028caf...406eac1

2023-07-25 [email protected] Roll Flutter from d7ed5dc to 9def8f6 (21 revisions) (flutter/packages#4561)
2023-07-25 [email protected] [webview_flutter_android][webview_flutter_wkwebview] Fixes bug where `PlatformWebViewWidget` doesn't rebuild when the controller changes (flutter/packages#4533)
2023-07-24 [email protected] [ci] Enable Android emulator-based tests (flutter/packages#4494)
2023-07-24 [email protected] [google_maps_flutter_web] Options to disable tilt controls and configure gesture handling (flutter/packages#3258)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
LouiseHsu pushed a commit to LouiseHsu/flutter that referenced this pull request Jul 31, 2023
flutter/packages@8028caf...406eac1

2023-07-25 [email protected] Roll Flutter from d7ed5dc to 9def8f6 (21 revisions) (flutter/packages#4561)
2023-07-25 [email protected] [webview_flutter_android][webview_flutter_wkwebview] Fixes bug where `PlatformWebViewWidget` doesn't rebuild when the controller changes (flutter/packages#4533)
2023-07-24 [email protected] [ci] Enable Android emulator-based tests (flutter/packages#4494)
2023-07-24 [email protected] [google_maps_flutter_web] Options to disable tilt controls and configure gesture handling (flutter/packages#3258)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
vashworth pushed a commit to vashworth/flutter that referenced this pull request Aug 2, 2023
flutter/packages@8028caf...406eac1

2023-07-25 [email protected] Roll Flutter from d7ed5dc to 9def8f6 (21 revisions) (flutter/packages#4561)
2023-07-25 [email protected] [webview_flutter_android][webview_flutter_wkwebview] Fixes bug where `PlatformWebViewWidget` doesn't rebuild when the controller changes (flutter/packages#4533)
2023-07-24 [email protected] [ci] Enable Android emulator-based tests (flutter/packages#4494)
2023-07-24 [email protected] [google_maps_flutter_web] Options to disable tilt controls and configure gesture handling (flutter/packages#3258)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: google_maps_flutter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants