[google_maps_flutter] Improve README and API doc comments #8560

filiph · 2025-02-04T15:38:29Z

There are some inconsistencies and anachronisms in the various READMEs and in the API docs. This PR attempts to address them.

Makes sure to link to platform-specific READMEs from the main README file.
Makes the READMEs more consistent (e.g. only leaving the bare minimum quick start for the main README, and leaving all platform-specific notes to platform READMEs).
Using the more correct “Not supported on all platforms” instead of “Supported on Android only” for functionality that works on both Android and the Web. Intentionally staying vague in API docs in order to make later maintenance easier. (Map marker on web doesn't respect custom hue flutter#161948 (comment))
Remove a doc comment from before null safety about asserting non-null.
Format the platform sections in the main README so that they’re proper lists, and so that all of them end with a link to the appropriate platform-specific README.

Issues:

Pre-launch Checklist

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

- Makes sure to link to platform-specific READMEs from the main README file. - Makes the READMEs more consistent (e.g. only leaving the bare minimum quick start for the main README, and leaving all platform-specific notes to platform READMEs). - Using the more correct “Not supported on all platforms” instead of “Supported on Android only” for functionality that works on both Android and the Web. Intentionally staying vague in API docs in order to make later maintenance easier. (flutter/flutter#161948 (comment)) - Remove a doc comment from before null safety about asserting non-null. - Format the platform sections in the main README so that they’re proper lists, and so that all of them end with a link to the appropriate platform-specific README.

filiph · 2025-02-04T15:56:56Z

I see that an automated CHANGELOG check is failing. It felt weird to be updating the otherwise-laconic CHANGELOGs with documentation changes but I'm happy to do that.

stuartmorgan-g · 2025-02-07T20:25:13Z

I see that an automated CHANGELOG check is failing.

See https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#faq

stuartmorgan-g · 2025-02-07T20:26:56Z

packages/google_maps_flutter/google_maps_flutter/README.md

@@ -32,84 +41,82 @@ For more details, see [Getting started with Google Maps Platform](https://develo

 1. Set the `minSdkVersion` in `android/app/build.gradle`:


You can just remove this whole bullet point while you are touching this; Flutter itself only supports 21+ now.

stuartmorgan-g · 2025-02-07T20:28:40Z

packages/google_maps_flutter/google_maps_flutter/lib/src/google_map.dart

@@ -261,6 +264,8 @@ class GoogleMap extends StatefulWidget {
  /// * The My Location button animates to focus on the user's current location
  /// if the user's location is currently known.
  ///
+  /// This feature is not present in the Google Maps SDK for the web.


Ideally we wouldn't reference a specific platform here, but the ship has sailed on that below so we can do this now, and revisit the whole thing in the future.

stuartmorgan-g · 2025-02-07T20:29:27Z

packages/google_maps_flutter/google_maps_flutter_android/README.md

@@ -78,6 +78,10 @@ Google Play the latest renderer will not be available and the legacy renderer wi
 WARNING: `AndroidMapRenderer.legacy` is known to crash apps and is no longer supported by the Google Maps team
 and therefore cannot be supported by the Flutter team.

+#### Cloud-based map styling


The previous heading is ##, so this should be ###.

stuartmorgan-g · 2025-02-07T20:30:53Z

packages/google_maps_flutter/google_maps_flutter_android/README.md

@@ -78,6 +78,10 @@ Google Play the latest renderer will not be available and the legacy renderer wi
 WARNING: `AndroidMapRenderer.legacy` is known to crash apps and is no longer supported by the Google Maps team
 and therefore cannot be supported by the Flutter team.

+#### Cloud-based map styling
+
+Cloud-based map styling works on Android only if `AndroidMapRenderer.latest` map renderer has been initialized.


I think people may read this as having to explicitly initialize the latest renderer, but in most cases it's the default now. How about "Cloud-based map styling is not supported with the legacy renderer."

This reverts commit 54f9819.

filiph · 2025-02-10T14:23:19Z

Addressed feedback and incremented versions using update-release-info. The CI/CD for changelog is now green but the pipeline's failing elsewhere, in Firebase Test Lab. Snip:

More details are available at [ https://console.firebase.google.com/project/flutter-infra-staging/testlab/histories/bh.8eb89ae93eb3eb36/matrices/7103368191579169772 ].
┌─────────┬────────────────────────┬─────────────────────┐
│ OUTCOME │    TEST_AXIS_VALUE     │     TEST_DETAILS    │
├─────────┼────────────────────────┼─────────────────────┤
│ Failed  │ panther-33-en-portrait │ Application crashed │
└─────────┴────────────────────────┴─────────────────────┘

I don't have access to https://console.firebase.google.com/project/flutter-infra-staging/testlab/histories/bh.8eb89ae93eb3eb36/matrices/7103368191579169772 to check what exactly went wrong. Did I fudge something? I believe I only changed documentation but I'm a bit nervous because the failure is at packages/google_maps_flutter/google_maps_flutter_android which is one of the packages I touched.

reidbaker · 2025-02-10T15:06:08Z

Logcat and video from the test attached.
The "overview" shows java.lang.IllegalStateException: Trying to call onSurfaceCreated with no current context

google-maps-crash.mp4

plugins_android_test_packages_google_maps_flutter_google_maps_flutter_android_unknown_build_3e8e411b-bfa1-4d48-8d13-793bec41437e_example_0_panther-33-en-portrait_logcat.txt

Have not dug in but the failure looks related.

filiph · 2025-02-10T15:45:48Z

I just went through the CL line by line and, FWIW, the only changes are:

README + CHANGELOG files
doc comments in .dart files
version bump

stuartmorgan-g · 2025-02-10T16:12:33Z

The "overview" shows java.lang.IllegalStateException: Trying to call onSurfaceCreated with no current context

This is longstanding flake, apparently in the Google Maps SDK itself: flutter/flutter#159731

stuartmorgan-g · 2025-02-10T16:14:13Z

which is one of the packages I touched.

The tooling in this repo is configured to only run tests on changed packages when the changes are restricted to packages (vs repo-wide configuration), so flake for a change like this will alway be from the packages you touched :)

stuartmorgan-g

LGTM

…ocs-new

filiph · 2025-02-24T13:48:08Z

Sorry about the delay. The versions and changelogs have changed under me and I haven't noticed, but now it's back on track and ready to be merged.

…lutter/packages#8560)

flutter/packages@5498d4d...c44c228 2025-02-24 engine-flutter-autoroll@skia.org Manual roll Flutter from 39b4951 to 911aa75 (56 revisions) (flutter/packages#8687) 2025-02-24 filiph@users.noreply.github.com [google_maps_flutter] Improve README and API doc comments (flutter/packages#8560) 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 flutter-ecosystem@google.com 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

packages/google_maps_flutter/google_maps_flutter/README.md

@jokerttu

…#8754) Two issues caught by @jokerttu after #8560 was already merged: - a typo ("iOS-specific" --> "web-specific") - a forgotten changelog entry

@jokerttu

…flutter#8754) Two issues caught by @jokerttu after flutter#8560 was already merged: - a typo ("iOS-specific" --> "web-specific") - a forgotten changelog entry

There are some inconsistencies and anachronisms in the various READMEs and in the API docs. This PR attempts to address them. - Makes sure to link to platform-specific READMEs from the main README file. - Makes the READMEs more consistent (e.g. only leaving the bare minimum quick start for the main README, and leaving all platform-specific notes to platform READMEs). - Using the more correct “Not supported on all platforms” instead of “Supported on Android only” for functionality that works on both Android and the Web. Intentionally staying vague in API docs in order to make later maintenance easier. (flutter/flutter#161948 (comment)) - Remove a doc comment from before null safety about asserting non-null. - Format the platform sections in the main README so that they’re proper lists, and so that all of them end with a link to the appropriate platform-specific README. Issues: - Fixes flutter/flutter#161954 - Related: flutter/flutter#161950, flutter/flutter#161948

@jokerttu

…flutter#8754) Two issues caught by @jokerttu after flutter#8560 was already merged: - a typo ("iOS-specific" --> "web-specific") - a forgotten changelog entry

There are some inconsistencies and anachronisms in the various READMEs and in the API docs. This PR attempts to address them. - Makes sure to link to platform-specific READMEs from the main README file. - Makes the READMEs more consistent (e.g. only leaving the bare minimum quick start for the main README, and leaving all platform-specific notes to platform READMEs). - Using the more correct “Not supported on all platforms” instead of “Supported on Android only” for functionality that works on both Android and the Web. Intentionally staying vague in API docs in order to make later maintenance easier. (flutter/flutter#161948 (comment)) - Remove a doc comment from before null safety about asserting non-null. - Format the platform sections in the main README so that they’re proper lists, and so that all of them end with a link to the appropriate platform-specific README. Issues: - Fixes flutter/flutter#161954 - Related: flutter/flutter#161950, flutter/flutter#161948

@jokerttu

…flutter#8754) Two issues caught by @jokerttu after flutter#8560 was already merged: - a typo ("iOS-specific" --> "web-specific") - a forgotten changelog entry

filiph added 2 commits February 4, 2025 14:26

Merge remote-tracking branch 'upstream/main'

ffd405b

filiph requested review from reidbaker, ditman and stuartmorgan-g as code owners February 4, 2025 15:38

github-actions bot added p: google_maps_flutter platform-android platform-web labels Feb 4, 2025

stuartmorgan-g requested changes Feb 7, 2025

View reviewed changes

filiph added 5 commits February 10, 2025 14:03

Address feedback

9660648

Add lines to CHANGELOG

54f9819

Merge remote-tracking branch 'upstream/main' into feat/maps-improve-docs

4e06db9

Revert "Add lines to CHANGELOG"

d1e0e72

This reverts commit 54f9819.

Use update-release-info to update versions and CHANGELOGs

09be9e3

filiph requested a review from stuartmorgan-g February 10, 2025 14:17

stuartmorgan-g approved these changes Feb 10, 2025

View reviewed changes

reidbaker approved these changes Feb 10, 2025

View reviewed changes

filiph and others added 4 commits February 24, 2025 14:00

Merge branch 'main' into feat/maps-improve-docs

e59276c

Merge remote-tracking branch 'upstream/main' into feat/maps-improve-d…

4ebd3b5

…ocs-new

Run update-release-info to update CHANGELOGs

6721f56

Merge branch 'feat/maps-improve-docs-new' into feat/maps-improve-docs

b5fce57

stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 24, 2025

auto-submit bot merged commit 4341341 into flutter:main Feb 24, 2025
82 checks passed

filiph deleted the feat/maps-improve-docs branch February 24, 2025 16:45

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 25, 2025

434134128 [google_maps_flutter] Improve README and API doc comments (f…

efc82fa

…lutter/packages#8560)

engine-flutter-autoroll mentioned this pull request Feb 25, 2025

Roll Packages from 5498d4d2eb48 to c44c2282aad1 (2 revisions) flutter/flutter#164103

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 25, 2025

434134128 [google_maps_flutter] Improve README and API doc comments (f…

3eeebb3

…lutter/packages#8560)

engine-flutter-autoroll mentioned this pull request Feb 25, 2025

Roll Packages from 5498d4d2eb48 to c44c2282aad1 (2 revisions) flutter/flutter#164121

Merged

jokerttu reviewed Feb 26, 2025

View reviewed changes

packages/google_maps_flutter/google_maps_flutter/README.md Show resolved Hide resolved

filiph mentioned this pull request Feb 28, 2025

[google_maps_flutter] Fix typo and remove duplicitous CHANGELOG entry #8754

Merged

11 tasks

		@@ -32,84 +41,82 @@ For more details, see [Getting started with Google Maps Platform](https://develo

		1. Set the `minSdkVersion` in `android/app/build.gradle`:

[google_maps_flutter] Improve README and API doc comments #8560

[google_maps_flutter] Improve README and API doc comments #8560

Uh oh!

Conversation

filiph commented Feb 4, 2025 • edited by stuartmorgan-g Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

filiph commented Feb 4, 2025

Uh oh!

stuartmorgan-g commented Feb 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

filiph commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

reidbaker commented Feb 10, 2025

Uh oh!

filiph commented Feb 10, 2025

Uh oh!

stuartmorgan-g commented Feb 10, 2025

Uh oh!

stuartmorgan-g commented Feb 10, 2025

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

filiph commented Feb 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

filiph commented Feb 4, 2025 •

edited by stuartmorgan-g

Loading

filiph commented Feb 10, 2025 •

edited

Loading