[google_map_flutter_ios] fix `didBeginDraggingMarker` typo #5085

cyanglaz · 2023-10-06T22:13:14Z

Fix the typo in didBeginDraggingMarker

The UITest will drag the marker then looking for the text indications that all the dragging callbacks are called, like the screenshot below:

fixes flutter/flutter#135778

Pre-launch Checklist

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

fix revert bundle id remove test

stuartmorgan

LGTM with some minor comments. Thanks for all the new test code here! It's crazy that this works even without the key set up. I guess it does everything but draw?

stuartmorgan · 2023-10-06T23:53:30Z

packages/google_maps_flutter/google_maps_flutter_ios/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 2.3.2
+
+* Fix an issue where the onDragEnd callback for marker is not called.


Nit: Fixes

(Per repo CHANGELOG style guide.)

stuartmorgan · 2023-10-06T23:54:32Z

...ogle_maps_flutter/google_maps_flutter_ios/example/ios13/ios/Runner.xcodeproj/project.pbxproj

+			isa = XCBuildConfiguration;
+			buildSettings = {
+				ASSETCATALOG_COMPILER_GENERATE_SWIFT_ASSET_SYMBOL_EXTENSIONS = YES;
+				CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE;


Most of these should be removable; see the list in https://github.com/flutter/flutter/wiki/Plugin-Tests#enabling-xctests-or-xcuitests

stuartmorgan · 2023-10-06T23:56:19Z

.../google_maps_flutter/google_maps_flutter_ios/example/ios13/ios/RunnerUITests/RunnerUITests.m

+
+#import <XCTest/XCTest.h>
+
+static const CGFloat kWaitTime = 15;


This should be much longer (maybe 60), since Flutter policy is to not have timeouts in tests, to avoid flake. It's really inconvenient to have no timeouts with the XCUITest APIs, so a very long value is our best option.

stuartmorgan · 2023-10-06T23:56:46Z

.../google_maps_flutter/google_maps_flutter_ios/example/ios13/ios/RunnerUITests/RunnerUITests.m

+
+#import <XCTest/XCTest.h>
+
+static const CGFloat kWaitTime = 15;


Nit: This should be an NSTimeInterval rather than a CGFloat, since that conveys unit information.

.../google_maps_flutter/google_maps_flutter_ios/example/ios13/ios/RunnerUITests/RunnerUITests.m

stuartmorgan · 2023-10-06T23:59:12Z

..._maps_flutter/google_maps_flutter_ios/example/shared/maps_example_dart/lib/place_marker.dart

@@ -80,13 +82,25 @@ class PlaceMarkerBodyState extends State<PlaceMarkerBody> {
  Future<void> _onMarkerDrag(MarkerId markerId, LatLng newPosition) async {
    setState(() {
      markerPosition = newPosition;
+      if (!_onDragXcodeUITestHelperText.contains('\n_onMarkerDrag called')) {
+        // _onMarkerDrag can be called multiple times during a single drag.
+        // Only log n_onMarkerDrag once per dragging action to reduce noises in UI.


Nit: n_ -> _ (looks like copypasta from the newline probably).

cyanglaz · 2023-10-07T16:14:23Z

I guess it does everything but draw?

Yes it probably does everything but draw. We could possibly add more XCUITests.

cyanglaz · 2023-10-07T16:34:56Z

...ogle_maps_flutter/google_maps_flutter_ios/example/ios13/ios/Runner.xcodeproj/project.pbxproj

+			isa = XCBuildConfiguration;
+			buildSettings = {
+				ASSETCATALOG_COMPILER_GENERATE_SWIFT_ASSET_SYMBOL_EXTENSIONS = YES;
+				CLANG_ENABLE_MODULES = YES;


We need this to import the google maps module.

cyanglaz · 2023-10-07T16:36:33Z

Updated the project.pbxproj file, @stuartmorgan Could you please double check to see if I'm missing anything?

(I also added a failure in the UITest to make sure the CI picked up the test and fail, I will revert that change before landing)

stuartmorgan

LGTM. There may be a couple more build properties that could be removed, but it's a short enough list at this point that it's fine if it's not absolutely minimal.

cyanglaz · 2023-10-10T02:02:48Z

The test failed so the ci did pick up the test. Reverting the intentional code that made the test fail and landing when green.

…lutter/packages#5085)

flutter/packages@7de08ec...4b483f2 2023-10-10 15619084+vashworth@users.noreply.github.com Run tests on either macOS 12 or macOS 13 (flutter/packages#5089) 2023-10-10 ychris@google.com [google_map_flutter_ios] fix `didBeginDraggingMarker` typo (flutter/packages#5085) 2023-10-09 stuartmorgan@google.com [process] Import the `process` package (flutter/packages#5095) 2023-10-09 hrkadam.92@gmail.com [go_router] Fixes missing state.extra in onException() (flutter/packages#5077) 2023-10-09 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.22.0 to 2.22.1 (flutter/packages#5094) 2023-10-09 engine-flutter-autoroll@skia.org Roll Flutter from 5207a30 to f52fe4f (18 revisions) (flutter/packages#5093) 2023-10-09 chrislangham@rightnow.org [platform] Example app for platform (flutter/packages#4834) 2023-10-09 49699333+dependabot[bot]@users.noreply.github.com Bump ossf/scorecard-action from 2.2.0 to 2.3.0 (flutter/packages#5091) 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,rmistry@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://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

) Fix the typo in `didBeginDraggingMarker` The UITest will drag the marker then looking for the text indications that all the dragging callbacks are called, like the screenshot below: <img width="388" alt="Screenshot 2023-10-06 at 3 36 46 PM" src="https://github.com/flutter/packages/assets/3756895/d9e78c99-bb1d-4f47-a0d4-2f699fa9f743"> fixes flutter/flutter#135778

t push -:wq!

57e7b53

fix revert bundle id remove test

cyanglaz requested a review from stuartmorgan October 6, 2023 22:13

github-actions bot added p: google_maps_flutter platform-ios labels Oct 6, 2023

stuartmorgan approved these changes Oct 7, 2023

View reviewed changes

Chris Yang added 2 commits October 7, 2023 09:33

review

aa83d3d

try to fail test to test ci

1f7cbc8

cyanglaz commented Oct 7, 2023

View reviewed changes

stuartmorgan approved these changes Oct 7, 2023

View reviewed changes

Chris Yang added 2 commits October 9, 2023 06:43

try fix ci

3a1cab1

revert failure testing code

c2e28cb

cyanglaz added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 10, 2023

auto-submit bot merged commit 158ec1f into flutter:main Oct 10, 2023
70 checks passed

engine-flutter-autoroll mentioned this pull request Oct 10, 2023

Roll Packages from 7de08ecd1cba to 4b483f242936 (8 revisions) flutter/flutter#136258

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 10, 2023

158ec1f [google_map_flutter_ios] fix didBeginDraggingMarker typo (f…

432228e

…lutter/packages#5085)

cyanglaz deleted the google_map_wrong_api branch October 10, 2023 16:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[google_map_flutter_ios] fix `didBeginDraggingMarker` typo #5085

[google_map_flutter_ios] fix `didBeginDraggingMarker` typo #5085

cyanglaz commented Oct 6, 2023 •

edited

Loading

stuartmorgan left a comment

stuartmorgan Oct 6, 2023

stuartmorgan Oct 6, 2023

stuartmorgan Oct 6, 2023

stuartmorgan Oct 6, 2023

stuartmorgan Oct 6, 2023

cyanglaz commented Oct 7, 2023 •

edited

Loading

cyanglaz Oct 7, 2023

cyanglaz commented Oct 7, 2023

stuartmorgan left a comment

cyanglaz commented Oct 10, 2023

		@@ -1,3 +1,7 @@
		## 2.3.2

		* Fix an issue where the onDragEnd callback for marker is not called.


		#import <XCTest/XCTest.h>

		static const CGFloat kWaitTime = 15;

[google_map_flutter_ios] fix didBeginDraggingMarker typo #5085

[google_map_flutter_ios] fix didBeginDraggingMarker typo #5085

Conversation

cyanglaz commented Oct 6, 2023 • edited Loading

Pre-launch Checklist

stuartmorgan left a comment

Choose a reason for hiding this comment

stuartmorgan Oct 6, 2023

Choose a reason for hiding this comment

stuartmorgan Oct 6, 2023

Choose a reason for hiding this comment

stuartmorgan Oct 6, 2023

Choose a reason for hiding this comment

stuartmorgan Oct 6, 2023

Choose a reason for hiding this comment

stuartmorgan Oct 6, 2023

Choose a reason for hiding this comment

cyanglaz commented Oct 7, 2023 • edited Loading

cyanglaz Oct 7, 2023

Choose a reason for hiding this comment

cyanglaz commented Oct 7, 2023

stuartmorgan left a comment

Choose a reason for hiding this comment

cyanglaz commented Oct 10, 2023

[google_map_flutter_ios] fix `didBeginDraggingMarker` typo #5085

[google_map_flutter_ios] fix `didBeginDraggingMarker` typo #5085

cyanglaz commented Oct 6, 2023 •

edited

Loading

cyanglaz commented Oct 7, 2023 •

edited

Loading