diff --git a/lib/flutter_skill.dart b/lib/flutter_skill.dart index 2e8355cf..e4026f17 100644 --- a/lib/flutter_skill.dart +++ b/lib/flutter_skill.dart @@ -1158,13 +1158,33 @@ class FlutterSkillBinding { // ==================== ELEMENT FINDING ==================== + /// Resolves [key] against widget keys first, then semantics. + /// + /// Matching only `ValueKey` leaves whole classes of widget + /// unreachable: design-system fields commonly wrap their inner editable in a + /// private `GlobalKey` and expose nothing but `Semantics(identifier: ...)`, + /// and keys built from ints or enums never matched at all. + /// + /// Candidates are collected in tiers so a real widget key always beats a + /// semantics annotation — a label is a human-readable string that can easily + /// collide with an unrelated widget's key. static Element? _findElementByKey(String key) { - final matches = []; + final byWidgetKey = []; + final bySemanticsIdentifier = []; + final bySemanticsLabel = []; + void visit(Element element) { final widget = element.widget; - if (widget.key is ValueKey && - (widget.key as ValueKey).value == key) { - matches.add(element); + final widgetKey = widget.key; + if (widgetKey is ValueKey && '${widgetKey.value}' == key) { + byWidgetKey.add(element); + } else if (widget is Semantics) { + final properties = widget.properties; + if (properties.identifier == key) { + bySemanticsIdentifier.add(element); + } else if (properties.label == key) { + bySemanticsLabel.add(element); + } } element.visitChildren(visit); } @@ -1174,7 +1194,12 @@ class FlutterSkillBinding { if (binding.rootElement != null) { visit(binding.rootElement!); } - return _preferTopmostMatch(matches); + + for (final tier in [byWidgetKey, bySemanticsIdentifier, bySemanticsLabel]) { + final match = _preferTopmostMatch(tier); + if (match != null) return match; + } + return null; } static Element? _findElementByText(String text) { @@ -1215,6 +1240,11 @@ class FlutterSkillBinding { return matches.last; } + /// Exposes key resolution so its tie-breaking rules can be pinned by tests. + @visibleForTesting + static Element? findElementByKeyForTesting(String key) => + _findElementByKey(key); + static Element? _findElement({String? key, String? text}) { if (key != null) return _findElementByKey(key); if (text != null) return _findElementByText(text); diff --git a/pubspec.yaml b/pubspec.yaml index ea72930e..68f82fa5 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -10,7 +10,10 @@ executables: environment: sdk: '>=3.0.0 <4.0.0' - flutter: ">=3.0.0" + # The declared ">=3.0.0" was never accurate. Highest actual requirement in + # the source is Color.withValues (3.27); element lookup by key adds + # SemanticsProperties.identifier (3.19) and WidgetsBinding.rootElement (3.9). + flutter: ">=3.27.0" dependencies: flutter: diff --git a/test/element_lookup_test.dart b/test/element_lookup_test.dart new file mode 100644 index 00000000..c3c096d3 --- /dev/null +++ b/test/element_lookup_test.dart @@ -0,0 +1,129 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_skill/flutter_skill.dart'; +import 'package:flutter_test/flutter_test.dart'; + +/// Tests for resolving an element from the `key` an agent supplies. +/// +/// Agents address widgets by a single opaque string, so lookup has to cover +/// more than `ValueKey`: design-system widgets often expose nothing but +/// `Semantics(identifier: ...)`, and keys are frequently built from ints or +/// enums. Matches also have to be ordered — a route pushed on top stays behind +/// the still-mounted route below it in a pre-order walk. +void main() { + Element? find(String key) => + FlutterSkillBinding.findElementByKeyForTesting(key); + + group('element lookup by key', () { + testWidgets('should resolve a ValueKey', (tester) async { + await tester.pumpWidget(const MaterialApp( + home: Text('hello', key: ValueKey('greeting')), + )); + + expect(find('greeting')?.widget, isA()); + }); + + testWidgets('should resolve a key whose value is not a String', + (tester) async { + // Keys built from ints or enums previously never matched, because lookup + // tested for ValueKey specifically. + await tester.pumpWidget(const MaterialApp( + home: Text('row', key: ValueKey(42)), + )); + + expect(find('42')?.widget, isA()); + }); + + testWidgets('should resolve a Semantics identifier', (tester) async { + // The design-system shape from the report: the editable is wrapped and + // its key is private, so the identifier is the only handle an agent has. + await tester.pumpWidget(MaterialApp( + home: Material( + child: Semantics( + identifier: 'email_field', + child: TextField(), + ), + ), + )); + + final element = find('email_field'); + expect(element, isNotNull); + expect(element!.widget, isA()); + + // Lookup must land somewhere that still contains the editable, otherwise + // enter_text cannot descend to it. + expect( + descendantEditableText(element), + isNotNull, + reason: 'enter_text resolves the EditableText below the match', + ); + }); + + testWidgets('should resolve a Semantics label', (tester) async { + await tester.pumpWidget(MaterialApp( + home: Semantics( + label: 'Submit order', + child: SizedBox(width: 10, height: 10), + ), + )); + + expect(find('Submit order')?.widget, isA()); + }); + + testWidgets('should prefer a widget key over a semantics label', + (tester) async { + // A label is human-readable text and can collide with an unrelated + // widget's key, so the real key has to win. + await tester.pumpWidget(MaterialApp( + home: Column( + children: [ + Semantics(label: 'submit', child: SizedBox(width: 1, height: 1)), + Text('Submit', key: ValueKey('submit')), + ], + ), + )); + + expect(find('submit')?.widget, isA()); + }); + + testWidgets('should prefer the topmost route when a key appears twice', + (tester) async { + // The background route stays mounted under a pushed route and is visited + // first, so returning the first match would tap an invisible widget. + final navigatorKey = GlobalKey(); + await tester.pumpWidget(MaterialApp( + navigatorKey: navigatorKey, + home: const Text('background', key: ValueKey('shared')), + )); + + navigatorKey.currentState!.push(MaterialPageRoute( + builder: (_) => const Text('foreground', key: ValueKey('shared')), + )); + await tester.pumpAndSettle(); + + expect((find('shared')!.widget as Text).data, 'foreground'); + }); + + testWidgets('should return null when nothing matches', (tester) async { + await tester.pumpWidget(const MaterialApp(home: Text('hello'))); + + expect(find('no_such_key'), isNull); + }); + }); +} + +/// Mirrors the descent `enter_text` performs from a matched element down to the +/// editable it wraps. +Element? descendantEditableText(Element root) { + Element? found; + void visit(Element element) { + if (found != null) return; + if (element.widget is EditableText) { + found = element; + return; + } + element.visitChildren(visit); + } + + visit(root); + return found; +}