From d621ee349c6c4f2a06c4004170f210c021b0cea4 Mon Sep 17 00:00:00 2001 From: charliewwdev Date: Tue, 1 Sep 2026 17:02:33 +0800 Subject: [PATCH 1/2] fix(binding): resolve elements by semantics identifier and non-String keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lookup by key only matched ValueKey, which left common widgets unreachable (issue #52, problem 3). Design-system fields wrap their inner editable in a private GlobalKey and expose nothing but Semantics(identifier: ...), so an agent had no handle on them at all — only the "focus the field first, then send an empty key" trick worked. Keys built from ints or enums never matched either. Collect candidates in tiers — widget key, then semantics identifier, then semantics label — and return the first non-empty tier. A real key has to win over a semantics annotation because a label is human-readable text that can collide with an unrelated widget's key. Within a tier the existing topmost-route preference still applies. Widening to `ValueKey` of any type is a superset of the old behaviour: the value is compared by its string form, so ValueKey resolves exactly as before. enter_text needs no change — it already descends from the matched element to the nearest EditableText, which is what makes a matched Semantics wrapper usable. Bumps the declared Flutter floor to 3.19, the first release with SemanticsProperties.identifier. The previous ">=3.0.0" was already inaccurate; WidgetsBinding.rootElement needs 3.9. --- lib/flutter_skill.dart | 40 +++++++++-- pubspec.yaml | 5 +- test/element_lookup_test.dart | 129 ++++++++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+), 6 deletions(-) create mode 100644 test/element_lookup_test.dart 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..b0974554 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -10,7 +10,10 @@ executables: environment: sdk: '>=3.0.0 <4.0.0' - flutter: ">=3.0.0" + # 3.19 is the first release with SemanticsProperties.identifier, which + # element lookup by key relies on. The previous ">=3.0.0" was already + # inaccurate — WidgetsBinding.rootElement needs 3.9. + flutter: ">=3.19.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; +} From 52ed8e0b472a0b9086419ef7be3879f2e6778b7f Mon Sep 17 00:00:00 2001 From: charliewwdev Date: Tue, 1 Sep 2026 17:11:06 +0800 Subject: [PATCH 2/2] fix: declare the Flutter floor the source actually requires MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Correcting the previous commit: 3.19 covers SemanticsProperties.identifier but not Color.withValues at lib/flutter_skill.dart:4552, which landed in 3.27. Verified by grepping the flutter/flutter tags — absent in 3.24.0, present in 3.27.0. This is also why `dart analyze` has been failing on main: CI pins FLUTTER_VERSION to 3.24.0, below what the source needs. --- pubspec.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pubspec.yaml b/pubspec.yaml index b0974554..68f82fa5 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -10,10 +10,10 @@ executables: environment: sdk: '>=3.0.0 <4.0.0' - # 3.19 is the first release with SemanticsProperties.identifier, which - # element lookup by key relies on. The previous ">=3.0.0" was already - # inaccurate — WidgetsBinding.rootElement needs 3.9. - flutter: ">=3.19.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: