[SuperEditor][SuperTextField][iOS] Allow animating caret blink (Resolves #2099)

[angelosilvestre]

[SuperEditor][SuperTextField][iOS] Allow animating caret blink. Resolves #2099

Currently, our blinking caret has an on/off visibility, it doesn't animates the visibility changes.

This PR adds a fade in/out transition when the caret switches from visible to invisible.

Some problems that I found:

• Although we have an option to use a Ticker to drive the visibility change, we are setting the _ticker to null when stopBlinking is called. It means that once the user types or places the caret somewhere, which causes stopBlinking to be called to momentary stop blinking, we are switching to the timer mode:

super_editor/super_text_layout/lib/src/infrastructure/blink_controller.dart

Lines 90 to 96 in 4c7472b

	void stopBlinking() {
	_isVisible = true; // If we're not blinking then we need to be visible
	if (_ticker != null) {
	// We're using a Ticker to blink. Stop it.
	_ticker?.stop();
	_ticker = null;

super_editor/super_text_layout/lib/src/infrastructure/blink_controller.dart

Lines 68 to 88 in 4c7472b

	void startBlinking() {
	if (!indeterminateAnimationsEnabled) {
	// Never animate a blink when the app/test wants to avoid
	// indeterminate animations.
	return;
	}
	if (_ticker != null) {
	// We're using a Ticker to blink. Restart it.
	_ticker!
	..stop()
	..start();
	} else {
	// We're using a Timer to blink. Restart it.
	_timer?.cancel();
	_timer = Timer(_flashPeriod, _blink);
	}
	_lastBlinkTime = Duration.zero;
	notifyListeners();
	}

• Fixing the previous issue creates an issue in some tests that check if the caret is blinking. Such tests were working before because we were always using the timer mode. When switching to ticker mode, there are some tests failing due to pumpAndSettleTimeout. The timeout happens because two methods used in those tests (tester.ime.backspace and tester.doubleTapInParagraph) internally call pumpAndSettle.

Open questions:

• How should we expose the configuration to animate the caret blink? Via SuperEditorIosControlsController ?
• Should we add parameters to tester.ime.backspace and tester.doubleTapInParagraph to avoid calling pumpAndSettle?

[angelosilvestre]

@matthew-carroll Can you take a look at the questions mentioned in the PR description?

[matthew-carroll]

Can you also add a point here about performance? The reason we originally got rid of animated blinking was to reduce Flutter re-renders.

[angelosilvestre]

Was the original performance issue really about re-renders? Or was it related to repaints? I'm wondering if using e RepaintBoundary in our overlayers would make a difference.

[matthew-carroll]

How can the fade duration be independent of the flash period?

[angelosilvestre]

The caret isn't constantly fading in and out. It stays visible or invisible during the flash period, then quickly fades in/out.

Should we just use half of the flash period?

[matthew-carroll]

Rather than sayign something is possible, please describe under which circumstances something happens or doesn't happen.

[angelosilvestre]

Updated.

[matthew-carroll]

Please also specify the alternative. It's possible a developer reads this and then asks "what does it mean to not animate the opacity?" - let's make the distinction between fading blinking vs on/off blinking.

[angelosilvestre]

Updates.

[matthew-carroll]

This Dart Doc comment makes it sound like this property is a permanent configuration on this controller, but in fact it looks like a momentary flag that's switched on and off to capture the fact that we're at a specific moment in time. Please try to replace this description with one that describes what this property is tracking, including when we expect it to flip from true to false, or false to true. Also, the name of this should probably start with _isSomething..., if it represents a temporary condition.

[angelosilvestre]

I tried to improve it a little bit.

[matthew-carroll]

Please add a comment to clarify the meaning of _isVisible in this calculation, because you're actually using it as a proxy to decide whether we're "fading in" or "fading out", and that's not immediately obvious.

[angelosilvestre]

Updated.

[matthew-carroll]

Should we add parameters to tester.ime.backspace and tester.doubleTapInParagraph to avoid calling pumpAndSettle?

My initial thought is, no. I think we should adjust the Super Editor side to play well with common test behaviors, rather than change the test behaviors to fit Super Editor.

How should we expose the configuration to animate the caret blink? Via SuperEditorIosControlsController ?

I wanna make sure I understand this question - you exposed the property from the controller already, right?

[angelosilvestre]

My initial thought is, no. I think we should adjust the Super Editor side to play well with common test behaviors, rather than change the test behaviors to fit Super Editor.

But how should we do that, if the ticker is constantly scheduling frames?

I wanna make sure I understand this question - you exposed the property from the controller already, right?

I exposed from the controller, but it still isn't configurable in a way that apps can enable or disable it. The BlinkController is instantiated by us.