Deprecate iOS13 style ActivityIndicator by ctrysbita · Pull Request #62741 · flutter/flutter

ctrysbita · 2020-08-02T06:45:16Z

Description

Deprecate iOS13 style CupertinoActivityIndicator.

Related Issues

Tests

No changes to tests.

Checklist

Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

No, no existing tests failed, so this is not a breaking change.
Yes, this is a breaking change. If not, delete the remainder of this section.
- I wrote a design doc: https://flutter.dev/go/template Replace this with a link to your design doc's short link
- I got input from the developer relations team, specifically from: Replace with the names of who gave advice
- I wrote a migration guide: https://flutter.dev/go/breaking-changes-template Replace this with a link to a pull request that adds the migration guide to https://flutter.dev/docs/release/breaking-changes

flutter-dashboard · 2020-08-02T06:45:19Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

flutter-dashboard · 2020-08-02T06:45:19Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

skia-gold · 2020-08-02T06:59:45Z

Gold has detected about 16 untriaged digest(s) on patchset 1.
View them at https://flutter-gold.skia.org/cl/github/62741

skia-gold · 2020-08-02T16:02:45Z

Gold has detected about 16 untriaged digest(s) on patchset 2.
View them at https://flutter-gold.skia.org/cl/github/62741

skia-gold · 2020-08-03T03:41:45Z

Gold has detected about 32 untriaged digest(s) on patchset 2.
View them at https://flutter-gold.skia.org/cl/github/62741

ctrysbita · 2020-08-03T05:06:25Z

Fine.
I cannot find the correct version number for dev branch XD
I've tried 1.21.0-1.0.pre and 1.21.0 but neither is correct.

justinmc · 2020-08-03T17:58:41Z

Suggested change

'This feature was deprecated after v1.21.0.'

'This feature was deprecated after v1.21.0-1.0.pre.'

Is this suggestion exactly what you tried before, and it still didn't work? What if you try 1.20.0-7.1.pre?

skia-gold · 2020-08-04T16:40:50Z

Gold has detected about 3 untriaged digest(s) on patchset 3.
View them at https://flutter-gold.skia.org/cl/github/62741

justinmc · 2020-08-05T17:15:27Z

Those skia gold changes are expected, since this PR changes the default appearance from iOS13 to iOS14 style. I've accepted the changes to the gold images.

Also, there seems to be a test failure. @ctrysbita Do you see this failure if you run the test locally?

Test failure output

══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure object was thrown running a test:
  Expected: Object or closure painting: 'a rounded rectangle with RRect: RRect.fromLTRBR(-10.0,
-50.0, 10.0, -100.0, 10.0)'
  Actual: _WidgetTypeFinder:<exactly one widget with type "CupertinoActivityIndicator" (ignoring
offstage widgets): CupertinoActivityIndicator(state: _CupertinoActivityIndicatorState#26bc8(ticker
inactive))>
   Which: did not match the pattern.
          It called drawRRect with RRect, RRect.fromLTRBR(-10.0, -33.3, 10.0, -100.0, 10.0), which
was not exactly the expected RRect (RRect.fromLTRBR(-10.0, -50.0, 10.0, -100.0, 10.0)).
          The stack of the offending call was:
            #0      TestRecordingCanvas.noSuchMethod
(file:///b/s/w/ir/k/flutter/packages/flutter/test/rendering/recording_canvas.dart:91:70)
            #1      TestRecordingCanvas.drawRRect
(file:///b/s/w/ir/k/flutter/packages/flutter/test/rendering/recording_canvas.dart:61:7)
            #2      _CupertinoActivityIndicatorPainter.paint
(package:flutter/src/cupertino/activity_indicator.dart:247:14)
            #3      RenderCustomPaint._paintWithPainter
(package:flutter/src/rendering/custom_paint.dart:533:13)
            #4      RenderCustomPaint.paint (package:flutter/src/rendering/custom_paint.dart:574:7)
            #5      TestRecordingPaintingContext.paintChild
(file:///b/s/w/ir/k/flutter/packages/flutter/test/rendering/recording_canvas.dart:105:11)
            #6      RenderProxyBoxMixin.paint (package:flutter/src/rendering/proxy_box.dart:133:15)
            #7      _evaluatePainter
(file:///b/s/w/ir/k/flutter/packages/flutter/test/rendering/mock_canvas.dart:518:20)
            #8      _TestRecordingCanvasMatcher.matches
(file:///b/s/w/ir/k/flutter/packages/flutter/test/rendering/mock_canvas.dart:535:12)
            #9      _expect (package:test_api/src/frontend/expect.dart:142:30)
            #10     expect (package:test_api/src/frontend/expect.dart:60:3)
            #11     expect (package:flutter_test/src/widget_tester.dart:376:3)
            #12     main.<anonymous closure>
(file:///b/s/w/ir/k/flutter/packages/flutter/test/cupertino/activity_indicator_test.dart:210:5)
            <asynchronous suspension>
            #13     main.<anonymous closure>
(file:///b/s/w/ir/k/flutter/packages/flutter/test/cupertino/activity_indicator_test.dart)
            #14     testWidgets.<anonymous closure>.<anonymous closure>
(package:flutter_test/src/widget_tester.dart:146:29)
            <asynchronous suspension>
            #15     testWidgets.<anonymous closure>.<anonymous closure>
(package:flutter_test/src/widget_tester.dart)
            #16     TestWidgetsFlutterBinding._runTestBody
(package:flutter_test/src/binding.dart:784:19)
            <asynchronous suspension>
            #19     TestWidgetsFlutterBinding._runTest
(package:flutter_test/src/binding.dart:764:14)
            #20     AutomatedTestWidgetsFlutterBinding.runTest.<anonymous closure>
(package:flutter_test/src/binding.dart:1173:24)
            #21     FakeAsync.run.<anonymous closure>.<anonymous closure>
(package:fake_async/fake_async.dart:177:54)
            #26     withClock (package:clock/src/default.dart:46:10)
            #27     FakeAsync.run.<anonymous closure> (package:fake_async/fake_async.dart:177:22)
            #32     FakeAsync.run (package:fake_async/fake_async.dart:177:7)
            #33     AutomatedTestWidgetsFlutterBinding.runTest
(package:flutter_test/src/binding.dart:1170:15)
            #34     testWidgets.<anonymous closure>
(package:flutter_test/src/widget_tester.dart:138:24)
            #35     Declarer.test.<anonymous closure>.<anonymous closure>
(package:test_api/src/backend/declarer.dart:171:19)
            <asynchronous suspension>
            #36     Declarer.test.<anonymous closure>.<anonymous closure>
(package:test_api/src/backend/declarer.dart)
            #41     Declarer.test.<anonymous closure>
(package:test_api/src/backend/declarer.dart:169:13)
            #42     Invoker.waitForOutstandingCallbacks.<anonymous closure>
(package:test_api/src/backend/invoker.dart:229:15)
            #47     Invoker.waitForOutstandingCallbacks
(package:test_api/src/backend/invoker.dart:226:5)
            #48     Invoker._onRun.<anonymous closure>.<anonymous closure>.<anonymous closure>
(package:test_api/src/backend/invoker.dart:381:17)
            <asynchronous suspension>
            #49     Invoker._onRun.<anonymous closure>.<anonymous closure>.<anonymous closure>
(package:test_api/src/backend/invoker.dart)
            #54     Invoker._onRun.<anonymous closure>.<anonymous closure>
(package:test_api/src/backend/invoker.dart:368:9)
            #55     Invoker._guardIfGuarded (package:test_api/src/backend/invoker.dart:413:15)
            #56     Invoker._onRun.<anonymous closure>
(package:test_api/src/backend/invoker.dart:367:7)
            #63     Invoker._onRun (package:test_api/src/backend/invoker.dart:366:11)
            #64     LiveTestController.run
(package:test_api/src/backend/live_test_controller.dart:152:11)
            #65     RemoteListener._runLiveTest.<anonymous closure>
(package:test_api/src/remote_listener.dart:259:16)
            #70     RemoteListener._runLiveTest (package:test_api/src/remote_listener.dart:258:5)
            #71     RemoteListener._serializeTest.<anonymous closure>
(package:test_api/src/remote_listener.dart:211:7)
            #89     _GuaranteeSink.add (package:stream_channel/src/guarantee_channel.dart:124:12)
            #90     new _MultiChannel.<anonymous closure>
(package:stream_channel/src/multi_channel.dart:159:31)
            #124    new _WebSocketImpl._fromSocket.<anonymous closure>
(dart:_http/websocket_impl.dart:1145:21)
            #132    _WebSocketProtocolTransformer._messageFrameEnd
(dart:_http/websocket_impl.dart:338:23)
            #133    _WebSocketProtocolTransformer.add (dart:_http/websocket_impl.dart:232:46)
            #143    _Socket._onData (dart:io-patch/socket_patch.dart:2043:41)
            #152    new _RawSocket.<anonymous closure> (dart:io-patch/socket_patch.dart:1579:33)
            #153    _NativeSocket.issueReadEvent.issue (dart:io-patch/socket_patch.dart:1075:14)
            (elided 108 frames from dart:async and package:stack_trace)
          The complete display list was:
            * save()
            * save()
            * translate(400.0, 300.0)
            * drawRRect(RRect.fromLTRBR(-10.0, -33.3, 10.0, -100.0, 10.0), Paint(Color(0x933c3c44)))
            * rotate(0.8)
            * drawRRect(RRect.fromLTRBR(-10.0, -33.3, 10.0, -100.0, 10.0), Paint(Color(0x933c3c44)))
            * rotate(0.8)
            * drawRRect(RRect.fromLTRBR(-10.0, -33.3, 10.0, -100.0, 10.0), Paint(Color(0x933c3c44)))
            * rotate(0.8)
            * drawRRect(RRect.fromLTRBR(-10.0, -33.3, 10.0, -100.0, 10.0), Paint(Color(0x933c3c44)))
            * rotate(0.8)
            * drawRRect(RRect.fromLTRBR(-10.0, -33.3, 10.0, -100.0, 10.0), Paint(Color(0x933c3c44)))
            * rotate(0.8)
            * drawRRect(RRect.fromLTRBR(-10.0, -33.3, 10.0, -100.0, 10.0), Paint(Color(0x933c3c44)))
            * rotate(0.8)
            * drawRRect(RRect.fromLTRBR(-10.0, -33.3, 10.0, -100.0, 10.0), Paint(Color(0x933c3c44)))
            * rotate(0.8)
            * drawRRect(RRect.fromLTRBR(-10.0, -33.3, 10.0, -100.0, 10.0), Paint(Color(0x933c3c44)))
            * rotate(0.8)
            * restore()
            * restore()

When the exception was thrown, this was the stack:
#4      main.<anonymous closure> (file:///b/s/w/ir/k/flutter/packages/flutter/test/cupertino/activity_indicator_test.dart:210:5)
<asynchronous suspension>
#5      main.<anonymous closure> (file:///b/s/w/ir/k/flutter/packages/flutter/test/cupertino/activity_indicator_test.dart)
#6      testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:146:29)
<asynchronous suspension>
#7      testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart)
#8      TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:784:19)
<asynchronous suspension>
#11     TestWidgetsFlutterBinding._runTest (package:flutter_test/src/binding.dart:764:14)
#12     AutomatedTestWidgetsFlutterBinding.runTest.<anonymous closure> (package:flutter_test/src/binding.dart:1173:24)
#13     FakeAsync.run.<anonymous closure>.<anonymous closure> (package:fake_async/fake_async.dart:177:54)
#18     withClock (package:clock/src/default.dart:46:10)
#19     FakeAsync.run.<anonymous closure> (package:fake_async/fake_async.dart:177:22)
#24     FakeAsync.run (package:fake_async/fake_async.dart:177:7)
#25     AutomatedTestWidgetsFlutterBinding.runTest (package:flutter_test/src/binding.dart:1170:15)
#26     testWidgets.<anonymous closure> (package:flutter_test/src/widget_tester.dart:138:24)
#27     Declarer.test.<anonymous closure>.<anonymous closure> (package:test_api/src/backend/declarer.dart:171:19)
<asynchronous suspension>
#28     Declarer.test.<anonymous closure>.<anonymous closure> (package:test_api/src/backend/declarer.dart)
#33     Declarer.test.<anonymous closure> (package:test_api/src/backend/declarer.dart:169:13)
#34     Invoker.waitForOutstandingCallbacks.<anonymous closure> (package:test_api/src/backend/invoker.dart:229:15)
#39     Invoker.waitForOutstandingCallbacks (package:test_api/src/backend/invoker.dart:226:5)
#40     Invoker._onRun.<anonymous closure>.<anonymous closure>.<anonymous closure> (package:test_api/src/backend/invoker.dart:381:17)
<asynchronous suspension>
#41     Invoker._onRun.<anonymous closure>.<anonymous closure>.<anonymous closure> (package:test_api/src/backend/invoker.dart)
#46     Invoker._onRun.<anonymous closure>.<anonymous closure> (package:test_api/src/backend/invoker.dart:368:9)
#47     Invoker._guardIfGuarded (package:test_api/src/backend/invoker.dart:413:15)
#48     Invoker._onRun.<anonymous closure> (package:test_api/src/backend/invoker.dart:367:7)
#55     Invoker._onRun (package:test_api/src/backend/invoker.dart:366:11)
#56     LiveTestController.run (package:test_api/src/backend/live_test_controller.dart:152:11)
#57     RemoteListener._runLiveTest.<anonymous closure> (package:test_api/src/remote_listener.dart:259:16)
#62     RemoteListener._runLiveTest (package:test_api/src/remote_listener.dart:258:5)
#63     RemoteListener._serializeTest.<anonymous closure> (package:test_api/src/remote_listener.dart:211:7)
#81     _GuaranteeSink.add (package:stream_channel/src/guarantee_channel.dart:124:12)
#82     new _MultiChannel.<anonymous closure> (package:stream_channel/src/multi_channel.dart:159:31)
#116    new _WebSocketImpl._fromSocket.<anonymous closure> (dart:_http/websocket_impl.dart:1145:21)
#124    _WebSocketProtocolTransformer._messageFrameEnd (dart:_http/websocket_impl.dart:338:23)
#125    _WebSocketProtocolTransformer.add (dart:_http/websocket_impl.dart:232:46)
#135    _Socket._onData (dart:io-patch/socket_patch.dart:2043:41)
#144    new _RawSocket.<anonymous closure> (dart:io-patch/socket_patch.dart:1579:33)
#145    _NativeSocket.issueReadEvent.issue (dart:io-patch/socket_patch.dart:1075:14)
(elided 108 frames from dart:async and package:stack_trace)

This was caught by the test expectation on the following line:
  file:///b/s/w/ir/k/flutter/packages/flutter/test/cupertino/activity_indicator_test.dart line 210
The test description was:
  has the correct corner radius
════════════════════════════════════════════════════════════════════════════════════════════════════

ctrysbita · 2020-08-05T22:40:02Z

@justinmc I'm working on fixing it.

justinmc · 2020-08-10T16:52:59Z

I think I've seen the current failure in another PR before (something about "forEach called on null"). Can you update the branch with master?

ctrysbita · 2020-08-11T02:20:42Z

@justinmc I've rebased my branch onto latest master.

justinmc

@ctrysbita Would you mind bringing back the deleted test that I mentioned in the comment below? Then I think this is ready to merge. Thanks for getting the tests to go green.

justinmc · 2020-08-11T20:31:15Z

    );
  });

-  testWidgets('Activity indicator with iOS14 style',


I think it's still valuable to keep this test around. Just leave off iOSVersionStyle so it defaults to iOS14?

The original tests are now default to iOS14 so we nolonger need these iOS14 tests.
But should we add some tests for iOS13 style?

Oh I see. No I think it's fine since iOS13 will be removed in the future.

Alright this looks good and should be merged when our build is green. Thanks for following up with this work!

fluttergithubbot · 2020-08-12T17:51:04Z

This pull request is not suitable for automatic merging in its current state.

Please get at least one approved review before re-applying this label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

justinmc

LGTM 👍

flutter-dashboard Bot added p: cupertino_ui cupertino_ui package in flutter/packages framework flutter/packages/flutter repository. See also f: labels. labels Aug 2, 2020

googlebot added the cla: yes label Aug 2, 2020

justinmc reviewed Aug 3, 2020

View reviewed changes

ctrysbita added 4 commits August 11, 2020 10:18

Deprecate iOS13 style ActivityIndicator

f72f5ad

Update deprecation annotation

dfa1aa7

Deprecate iOSVersionStyle field

34e690d

Update tests

1f908f4

justinmc reviewed Aug 11, 2020

View reviewed changes

justinmc added the waiting for tree to go green label Aug 12, 2020

fluttergithubbot removed the waiting for tree to go green label Aug 12, 2020

justinmc approved these changes Aug 12, 2020

View reviewed changes

justinmc added the waiting for tree to go green label Aug 12, 2020

fluttergithubbot merged commit 7d1842b into flutter:master Aug 12, 2020

mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020

Deprecate iOS13 style ActivityIndicator (flutter#62741)

f847b1d

jmagman mentioned this pull request Sep 15, 2020

Deprecate iOSVersionStyle.iOS13 #62521

Closed

DanielEdrisian mentioned this pull request Oct 16, 2020

Add back iOSVersionStyle option to CupertinoActivityIndicator to allow catering to iOS13 and iOS14 #68285

Closed

github-actions Bot locked as resolved and limited conversation to collaborators Aug 12, 2021

	'This feature was deprecated after v1.21.0.'
	'This feature was deprecated after v1.21.0-1.0.pre.'

Conversation

ctrysbita commented Aug 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

flutter-dashboard Bot commented Aug 2, 2020

Uh oh!

flutter-dashboard Bot commented Aug 2, 2020

Uh oh!

skia-gold commented Aug 2, 2020

Uh oh!

skia-gold commented Aug 2, 2020

Uh oh!

skia-gold commented Aug 3, 2020

Uh oh!

ctrysbita commented Aug 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

justinmc Aug 3, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

skia-gold commented Aug 4, 2020

Uh oh!

justinmc commented Aug 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ctrysbita commented Aug 5, 2020

Uh oh!

justinmc commented Aug 10, 2020

Uh oh!

ctrysbita commented Aug 11, 2020

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc Aug 11, 2020

Choose a reason for hiding this comment

Uh oh!

ctrysbita Aug 12, 2020

Choose a reason for hiding this comment

Uh oh!

justinmc Aug 12, 2020

Choose a reason for hiding this comment

Uh oh!

fluttergithubbot commented Aug 12, 2020

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ctrysbita commented Aug 2, 2020 •

edited

Loading

ctrysbita commented Aug 3, 2020 •

edited

Loading

justinmc commented Aug 5, 2020 •

edited

Loading