TextSelectionHandleControls deprecation deletion timeframe

[justinmc]

This came out of this discussion on another PR. This PR executes "Option 2" in my list of options.

Currently, TextSelectionControls.buildToolbar and TextSelectionHandleControls have the same deprecation version, which implies that they will both be deleted at the same time. This creates a problematic migration for users:

1. Receive a deprecation warning. Migrate from TextSelectionControls.buildToolbar to EditableText.buildContextMenuand fromTextSelectionControls.buildHandletoTextSelectionHandleControls.buildHandle`.
2. Receive a sudden breakage when TextSelectionHandleControls is deleted, and have to migrate immediately back to TextSelectionControls.buildHandle.

Instead, I've bumped the deprecation version so that the deletion happens in two stages. This results in a two-stage migration process and avoids the sudden breaking change:

1. (Same as before) Receive a deprecation warning. Migrate from TextSelectionControls.buildToolbar to EditableText.buildContextMenu and from TextSelectionControls.buildHandle to TextSelectionHandleControls.buildHandle.
2. Receive another deprecation warning when TextSelectionControls.buildToolbar is removed. Migrate from TextSelectionHandleControls.buildToolbar back to TextSelectionControls.buildToolbar.
3. TextSelectionHandleControls is removed safely.

[flutter-dashboard]

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 (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

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

[Piinks]

I think this SGTM. If we need to extend the other deprecations further, we can always do that too. Later even if we decide it is needed.

Are there any dart fixes for these? I don't think dart fix can help with replacing mixins, but there could be some assistance from dart fix for the other related APIs that are deprecated for this.

We learned when the typography changes from 2014 (A) were replaced by the 2018 (B) and then the current (C), dart fix will actually migrate A directly to C, skipping B. :)

[gnprice]

Instead, I've bumped the deprecation version so that the deletion happens in two stages. This results in a two-stage migration process and avoids the sudden breaking change

This sounds great.

I wonder if it'd be helpful to go a step farther and delete the @Deprecated annotation on this class entirely. If I'm understanding correctly, any deprecation warning that it causes is not going to be actionable right now — this is version B of the API, and C isn't yet available. Then when adding C, we can add back this @Deprecated annotation.

There could still be text in the class's doc explaining that the plan is this will be removed in the future after C is available. But by removing the formal annotation, any developers who see the strikethrough on this class's name in their IDE when they use it won't be frustrated wondering what it is they're supposed to do instead. Nor have to keep looking at the strikethrough if they're trying to keep their project all clean and up to date 🙂

[justinmc]

Good point! I agree, I'll remove the deprecation altogether and replace it with a TODO for myself to deprecate it later.

[justinmc]

@Piinks There are no dart fixes. The simplest thing would be helping to migrate from TextSelectionControls.buildToolbar to TextField.contextMenuBuilder (or CupertinoTextField or TextFormField). But this deprecation has already been released on stable, so I'm guessing most users have already migrated (A to B).

[gnprice]

LGTM!

[Hixie]

test-exempt: annotation change

...but I have to say, this doesn't seem like the most fun for developers. I don't really understand the situation in detail but it seems like we should find some way to automigrate people to the final API in one go, and not have them worry about any of this at all.

[justinmc]

I agree, this one is my bad. I should have just deprecated all of TextSelectionControls and permanently moved everyone to TextSelectionHandleControls in one step. I thought I could be clever with this 2 step approach since it only should affect a small number of users, but it's too complicated.

Anyway, this PR will make it a little better by at least having two clear, separate migration steps.