Unverified Commit 93fc7f59 authored by chunhtai's avatar chunhtai Committed by GitHub

fix hidden TextSpan with recognizer does not auto scroll (#100494)

parent 2e357879
......@@ -4,6 +4,8 @@
import 'package:meta/meta.dart';
import 'diagnostics.dart';
/// A [Key] is an identifier for [Widget]s, [Element]s and [SemanticsNode]s.
///
/// A new widget will only be used to update an existing element if its key is
......@@ -47,6 +49,23 @@ abstract class LocalKey extends Key {
const LocalKey() : super.empty();
}
/// A key that is only equal to itself.
///
/// This cannot be created with a const constructor because that implies that
/// all instantiated keys would be the same instance and therefore not be unique.
class UniqueKey extends LocalKey {
/// Creates a key that is equal only to itself.
///
/// The key cannot be created with a const constructor because that implies
/// that all instantiated keys would be the same instance and therefore not
/// be unique.
// ignore: prefer_const_constructors_in_immutables , never use const for this class
UniqueKey();
@override
String toString() => '[#${shortHash(this)}]';
}
/// A key that uses a value of a particular type to identify itself.
///
/// A [ValueKey<T>] is equal to another [ValueKey<T>] if, and only if, their
......
......@@ -1222,7 +1222,7 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
// can be re-used when [assembleSemanticsNode] is called again. This ensures
// stable ids for the [SemanticsNode]s of [TextSpan]s across
// [assembleSemanticsNode] invocations.
Queue<SemanticsNode>? _cachedChildNodes;
LinkedHashMap<Key, SemanticsNode>? _cachedChildNodes;
/// Returns a list of rects that bound the given selection.
///
......@@ -1326,7 +1326,7 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
int placeholderIndex = 0;
int childIndex = 0;
RenderBox? child = firstChild;
final Queue<SemanticsNode> newChildCache = Queue<SemanticsNode>();
final LinkedHashMap<Key, SemanticsNode> newChildCache = LinkedHashMap<Key, SemanticsNode>();
_cachedCombinedSemanticsInfos ??= combineSemanticsInfo(_semanticsInfo!);
for (final InlineSpanSemanticsInformation info in _cachedCombinedSemanticsInfos!) {
final TextSelection selection = TextSelection(
......@@ -1406,13 +1406,24 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
assert(false, '${recognizer.runtimeType} is not supported.');
}
}
final SemanticsNode newChild = (_cachedChildNodes?.isNotEmpty ?? false)
? _cachedChildNodes!.removeFirst()
: SemanticsNode();
if (node.parentPaintClipRect != null) {
final Rect paintRect = node.parentPaintClipRect!.intersect(currentRect);
configuration.isHidden = paintRect.isEmpty && !currentRect.isEmpty;
}
late final SemanticsNode newChild;
if (_cachedChildNodes?.isNotEmpty ?? false) {
newChild = _cachedChildNodes!.remove(_cachedChildNodes!.keys.first)!;
} else {
final UniqueKey key = UniqueKey();
newChild = SemanticsNode(
key: key,
showOnScreen: _createShowOnScreenFor(key),
);
}
newChild
..updateWith(config: configuration)
..rect = currentRect;
newChildCache.addLast(newChild);
newChildCache[newChild.key!] = newChild;
newChildren.add(newChild);
}
}
......@@ -1420,6 +1431,13 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
node.updateWith(config: config, childrenInInversePaintOrder: newChildren);
}
VoidCallback? _createShowOnScreenFor(Key key) {
return () {
final SemanticsNode node = _cachedChildNodes![key]!;
showOnScreen(descendant: this, rect: node.rect);
};
}
// TODO(ianh): in theory, [selection] could become null between when
// we last called describeSemanticsConfiguration and when the
// callbacks are invoked, in which case the callbacks will crash...
......
......@@ -908,7 +908,7 @@ class RenderParagraph extends RenderBox
// can be re-used when [assembleSemanticsNode] is called again. This ensures
// stable ids for the [SemanticsNode]s of [TextSpan]s across
// [assembleSemanticsNode] invocations.
Queue<SemanticsNode>? _cachedChildNodes;
LinkedHashMap<Key, SemanticsNode>? _cachedChildNodes;
@override
void assembleSemanticsNode(SemanticsNode node, SemanticsConfiguration config, Iterable<SemanticsNode> children) {
......@@ -921,7 +921,7 @@ class RenderParagraph extends RenderBox
int placeholderIndex = 0;
int childIndex = 0;
RenderBox? child = firstChild;
final Queue<SemanticsNode> newChildCache = Queue<SemanticsNode>();
final LinkedHashMap<Key, SemanticsNode> newChildCache = LinkedHashMap<Key, SemanticsNode>();
_cachedCombinedSemanticsInfos ??= combineSemanticsInfo(_semanticsInfo!);
for (final InlineSpanSemanticsInformation info in _cachedCombinedSemanticsInfos!) {
final TextSelection selection = TextSelection(
......@@ -1004,13 +1004,24 @@ class RenderParagraph extends RenderBox
assert(false, '${recognizer.runtimeType} is not supported.');
}
}
final SemanticsNode newChild = (_cachedChildNodes?.isNotEmpty ?? false)
? _cachedChildNodes!.removeFirst()
: SemanticsNode();
if (node.parentPaintClipRect != null) {
final Rect paintRect = node.parentPaintClipRect!.intersect(currentRect);
configuration.isHidden = paintRect.isEmpty && !currentRect.isEmpty;
}
late final SemanticsNode newChild;
if (_cachedChildNodes?.isNotEmpty ?? false) {
newChild = _cachedChildNodes!.remove(_cachedChildNodes!.keys.first)!;
} else {
final UniqueKey key = UniqueKey();
newChild = SemanticsNode(
key: key,
showOnScreen: _createShowOnScreenFor(key),
);
}
newChild
..updateWith(config: configuration)
..rect = currentRect;
newChildCache.addLast(newChild);
newChildCache[newChild.key!] = newChild;
newChildren.add(newChild);
}
}
......@@ -1022,6 +1033,13 @@ class RenderParagraph extends RenderBox
node.updateWith(config: config, childrenInInversePaintOrder: newChildren);
}
VoidCallback? _createShowOnScreenFor(Key key) {
return () {
final SemanticsNode node = _cachedChildNodes![key]!;
showOnScreen(descendant: this, rect: node.rect);
};
}
@override
void clearSemantics() {
super.clearSemantics();
......
......@@ -54,23 +54,6 @@ const _DebugOnly _debugOnly = _DebugOnly();
// KEYS
/// A key that is only equal to itself.
///
/// This cannot be created with a const constructor because that implies that
/// all instantiated keys would be the same instance and therefore not be unique.
class UniqueKey extends LocalKey {
/// Creates a key that is equal only to itself.
///
/// The key cannot be created with a const constructor because that implies
/// that all instantiated keys would be the same instance and therefore not
/// be unique.
// ignore: prefer_const_constructors_in_immutables , never use const for this class
UniqueKey();
@override
String toString() => '[#${shortHash(this)}]';
}
/// A key that takes its identity from the object used as its value.
///
/// Used to tie the identity of a widget to the identity of an object used to
......
......@@ -15,6 +15,7 @@ library widgets;
export 'package:characters/characters.dart';
export 'package:vector_math/vector_math_64.dart' show Matrix4;
export 'foundation.dart' show UniqueKey;
export 'src/widgets/actions.dart';
export 'src/widgets/animated_cross_fade.dart';
export 'src/widgets/animated_list.dart';
......
......@@ -2323,6 +2323,99 @@ void main() {
semantics.dispose();
});
testWidgets('semantic nodes of offscreen recognizers are marked hidden', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/100395.
final SemanticsTester semantics = SemanticsTester(tester);
const TextStyle textStyle = TextStyle(fontFamily: 'Ahem', fontSize: 200);
const String onScreenText = 'onscreen\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n';
const String offScreenText = 'off screen';
final ScrollController controller = ScrollController();
await tester.pumpWidget(
MaterialApp(
home: SingleChildScrollView(
controller: controller,
child: SelectableText.rich(
TextSpan(
children: <TextSpan>[
const TextSpan(text: onScreenText),
TextSpan(
text: offScreenText,
recognizer: TapGestureRecognizer()..onTap = () { },
),
],
style: textStyle,
),
textDirection: TextDirection.ltr,
),
)
),
);
final TestSemantics expectedSemantics = TestSemantics.root(
children: <TestSemantics>[
TestSemantics(
textDirection: TextDirection.ltr,
children: <TestSemantics>[
TestSemantics(
children: <TestSemantics>[
TestSemantics(
flags: <SemanticsFlag>[SemanticsFlag.scopesRoute],
children: <TestSemantics>[
TestSemantics(
flags: <SemanticsFlag>[SemanticsFlag.hasImplicitScrolling],
actions: <SemanticsAction>[SemanticsAction.scrollUp],
children: <TestSemantics>[
TestSemantics(
actions: <SemanticsAction>[SemanticsAction.longPress],
children: <TestSemantics>[
TestSemantics(
children: <TestSemantics>[
TestSemantics(
label: 'onscreen\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n',
textDirection: TextDirection.ltr,
),
TestSemantics(
flags: <SemanticsFlag>[
SemanticsFlag.isHidden,
SemanticsFlag.isLink
],
actions: <SemanticsAction>[SemanticsAction.tap],
label: 'off screen',
textDirection: TextDirection.ltr,
),
],
),
],
),
],
),
],
),
],
),
],
),
],
);
expect(
semantics,
hasSemantics(
expectedSemantics,
ignoreTransform: true,
ignoreId: true,
ignoreRect: true,
),
);
// Test show on screen.
expect(controller.offset, 0.0);
tester.binding.pipelineOwner.semanticsOwner!.performAction(8, SemanticsAction.showOnScreen);
await tester.pumpAndSettle();
expect(controller.offset != 0.0, isTrue);
semantics.dispose();
});
testWidgets('SelectableText change selection with semantics', (WidgetTester tester) async {
final SemanticsTester semantics = SemanticsTester(tester);
final SemanticsOwner semanticsOwner = tester.binding.pipelineOwner.semanticsOwner!;
......
......@@ -412,6 +412,75 @@ void main() {
semantics.dispose();
});
testWidgets('semantic nodes of offscreen recognizers are marked hidden', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/100395.
final SemanticsTester semantics = SemanticsTester(tester);
const TextStyle textStyle = TextStyle(fontFamily: 'Ahem', fontSize: 200);
const String onScreenText = 'onscreen\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n';
const String offScreenText = 'off screen';
final ScrollController controller = ScrollController();
await tester.pumpWidget(
SingleChildScrollView(
controller: controller,
child: Text.rich(
TextSpan(
children: <TextSpan>[
const TextSpan(text: onScreenText),
TextSpan(
text: offScreenText,
recognizer: TapGestureRecognizer()..onTap = () { },
),
],
style: textStyle,
),
textDirection: TextDirection.ltr,
),
),
);
final TestSemantics expectedSemantics = TestSemantics.root(
children: <TestSemantics>[
TestSemantics(
flags: <SemanticsFlag>[SemanticsFlag.hasImplicitScrolling],
actions: <SemanticsAction>[SemanticsAction.scrollUp],
children: <TestSemantics>[
TestSemantics(
children: <TestSemantics>[
TestSemantics(
label: onScreenText,
textDirection: TextDirection.ltr,
),
TestSemantics(
label: offScreenText,
textDirection: TextDirection.ltr,
actions: <SemanticsAction>[SemanticsAction.tap],
flags: <SemanticsFlag>[SemanticsFlag.isLink, SemanticsFlag.isHidden],
),
],
),
],
),
],
);
expect(
semantics,
hasSemantics(
expectedSemantics,
ignoreTransform: true,
ignoreId: true,
ignoreRect: true,
),
);
// Test show on screen.
expect(controller.offset, 0.0);
tester.binding.pipelineOwner.semanticsOwner!.performAction(4, SemanticsAction.showOnScreen);
await tester.pumpAndSettle();
expect(controller.offset != 0.0, isTrue);
semantics.dispose();
});
testWidgets('recognizers split semantic node when TextSpan overflows', (WidgetTester tester) async {
final SemanticsTester semantics = SemanticsTester(tester);
const TextStyle textStyle = TextStyle(fontFamily: 'Ahem');
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment