flutter / flutter

Flutter makes it easy and fast to build beautiful apps for mobile and beyond
https://flutter.dev
BSD 3-Clause "New" or "Revised" License
162.18k stars 26.64k forks source link

improve focus example #147464

Closed NobodyForNothing closed 1 week ago

NobodyForNothing commented 2 weeks ago

Part of #130459. Adds a test to the second focus example and makes the function of the example more clear.

Pre-launch Checklist

A111one commented 2 weeks ago

Welcome

On Wed, 1 May 2024, 3:27 am Justin McCandless, @.***> wrote:

@.**** approved this pull request.

LGTM with nits 👍

Thanks for helping to test these examples!

In examples/api/test/widgets/focus_scope/focus.1_test.dart https://github.com/flutter/flutter/pull/147464#discussion_r1585609112:

+import 'package:flutter_api_samples/widgets/focus_scope/focus.1.dart'

  • as example; +import 'package:flutter_test/flutter_test.dart';
  • +void main() {

  • testWidgets('FocusableText shows content and color depending on focus',
  • (WidgetTester tester) async {
  • await tester.pumpWidget(const MaterialApp(
  • home: Scaffold(
  • body: example.FocusableText(
  • 'Item 0',
  • autofocus: false,
  • ),
  • ),
  • ));
  • // Autofocus needs to check no other node in the [FocusScope] is focused and

Super nit for readability: "needs to check no" => "needs to check that no"

In examples/api/test/widgets/focus_scope/focus.1_test.dart https://github.com/flutter/flutter/pull/147464#discussion_r1585609479:

  • });
  • testWidgets('builds list showcasing focus traversal',

Needs a newline here between the tests.

— Reply to this email directly, view it on GitHub https://github.com/flutter/flutter/pull/147464#pullrequestreview-2032682577, or unsubscribe https://github.com/notifications/unsubscribe-auth/BH7GFSCGO5HQ3KUHJYER533ZAALFLAVCNFSM6AAAAABG4DJH6KVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMZSGY4DENJXG4 . You are receiving this because you commented.Message ID: @.***>

NobodyForNothing commented 1 week ago

Can someone rerun Google testing?

gspencergoog commented 1 week ago

Can someone rerun Google testing?

Done, but it looks like there are merge conflicts anyhow.