Skip to content

Commit

Permalink
Move Feedback to widgets layer (flutter#148523)
Browse files Browse the repository at this point in the history
Currently, `Feedback` exists in the Material layer. However, not only is
`Feedback` not material-opinionated, but it is an abstract class that
defines its functionality depending on the user's platform.

It makes sense that `Feedback` should exist in the widgets layer
instead. This makes it easier to incorporate platform specific
`Feedback` updates as they arrive, fixing issues like flutter#148391.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
[Data Driven Fixes]:
https://github.com/flutter/flutter/wiki/Data-driven-Fixes
  • Loading branch information
victorsanni authored May 18, 2024
1 parent adc9307 commit f7012db
Show file tree
Hide file tree
Showing 28 changed files with 37 additions and 43 deletions.
1 change: 0 additions & 1 deletion packages/flutter/lib/material.dart
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ export 'src/material/expand_icon.dart';
export 'src/material/expansion_panel.dart';
export 'src/material/expansion_tile.dart';
export 'src/material/expansion_tile_theme.dart';
export 'src/material/feedback.dart';
export 'src/material/filled_button.dart';
export 'src/material/filled_button_theme.dart';
export 'src/material/filter_chip.dart';
Expand Down
1 change: 0 additions & 1 deletion packages/flutter/lib/src/material/ink_well.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import 'package:flutter/rendering.dart';
import 'package:flutter/widgets.dart';

import 'debug.dart';
import 'feedback.dart';
import 'ink_highlight.dart';
import 'material.dart';
import 'material_state.dart';
Expand Down
1 change: 0 additions & 1 deletion packages/flutter/lib/src/material/selectable_text.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import 'package:flutter/rendering.dart';

import 'adaptive_text_selection_toolbar.dart';
import 'desktop_text_selection.dart';
import 'feedback.dart';
import 'magnifier.dart';
import 'text_selection.dart';
import 'theme.dart';
Expand Down
1 change: 0 additions & 1 deletion packages/flutter/lib/src/material/text_field.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import 'color_scheme.dart';
import 'colors.dart';
import 'debug.dart';
import 'desktop_text_selection.dart';
import 'feedback.dart';
import 'input_decorator.dart';
import 'magnifier.dart';
import 'material_localizations.dart';
Expand Down
1 change: 0 additions & 1 deletion packages/flutter/lib/src/material/time_picker.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import 'colors.dart';
import 'curves.dart';
import 'debug.dart';
import 'dialog.dart';
import 'feedback.dart';
import 'icon_button.dart';
import 'icons.dart';
import 'ink_well.dart';
Expand Down
1 change: 0 additions & 1 deletion packages/flutter/lib/src/material/tooltip.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import 'package:flutter/services.dart';
import 'package:flutter/widgets.dart';

import 'colors.dart';
import 'feedback.dart';
import 'text_theme.dart';
import 'theme.dart';
import 'tooltip_theme.dart';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:flutter/foundation.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/semantics.dart';
import 'package:flutter/services.dart';
import 'package:flutter/widgets.dart';

import 'theme.dart';
import 'framework.dart';
import 'gesture_detector.dart';

/// Provides platform-specific acoustic and/or haptic feedback for certain
/// actions.
Expand Down Expand Up @@ -92,7 +93,7 @@ abstract final class Feedback {
/// [GestureTapCallback].
static Future<void> forTap(BuildContext context) async {
context.findRenderObject()!.sendSemanticsEvent(const TapSemanticEvent());
switch (_platform(context)) {
switch (defaultTargetPlatform) {
case TargetPlatform.android:
case TargetPlatform.fuchsia:
return SystemSound.play(SystemSoundType.click);
Expand All @@ -119,7 +120,7 @@ abstract final class Feedback {
return null;
}
return () {
Feedback.forTap(context);
forTap(context);
callback();
};
}
Expand All @@ -135,7 +136,7 @@ abstract final class Feedback {
/// executing a [GestureLongPressCallback].
static Future<void> forLongPress(BuildContext context) {
context.findRenderObject()!.sendSemanticsEvent(const LongPressSemanticsEvent());
switch (_platform(context)) {
switch (defaultTargetPlatform) {
case TargetPlatform.android:
case TargetPlatform.fuchsia:
return HapticFeedback.vibrate();
Expand Down Expand Up @@ -167,6 +168,4 @@ abstract final class Feedback {
callback();
};
}

static TargetPlatform _platform(BuildContext context) => Theme.of(context).platform;
}
1 change: 1 addition & 0 deletions packages/flutter/lib/widgets.dart
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export 'src/widgets/draggable_scrollable_sheet.dart';
export 'src/widgets/dual_transition_builder.dart';
export 'src/widgets/editable_text.dart';
export 'src/widgets/fade_in_image.dart';
export 'src/widgets/feedback.dart';
export 'src/widgets/focus_manager.dart';
export 'src/widgets/focus_scope.dart';
export 'src/widgets/focus_traversal.dart';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';
import 'package:vector_math/vector_math_64.dart' show Vector3;

import '../widgets/feedback_tester.dart';
import '../widgets/semantics_tester.dart';
import 'feedback_tester.dart';

void main() {
testWidgets('BottomNavigationBar callback test', (WidgetTester tester) async {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';

import 'feedback_tester.dart';
import '../widgets/feedback_tester.dart';

void main() {
TestWidgetsFlutterBinding.ensureInitialized();
Expand Down
3 changes: 2 additions & 1 deletion packages/flutter/test/material/checkbox_list_tile_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import 'package:flutter/gestures.dart';
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter_test/flutter_test.dart';
import 'feedback_tester.dart';

import '../widgets/feedback_tester.dart';

Widget wrap({ required Widget child }) {
return MediaQuery(
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/material/chip_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/scheduler.dart';
import 'package:flutter_test/flutter_test.dart';
import '../widgets/feedback_tester.dart';
import '../widgets/semantics_tester.dart';
import 'feedback_tester.dart';

Finder findRenderChipElement() {
return find.byElementPredicate((Element e) => '${e.renderObject.runtimeType}' == '_RenderChip');
Expand Down
3 changes: 2 additions & 1 deletion packages/flutter/test/material/date_range_picker_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
import 'feedback_tester.dart';

import '../widgets/feedback_tester.dart';

void main() {
late DateTime firstDate;
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/material/dropdown_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';

import '../widgets/feedback_tester.dart';
import '../widgets/semantics_tester.dart';
import 'feedback_tester.dart';

const List<String> menuItems = <String>['one', 'two', 'three', 'four'];
void onChanged<T>(T _) { }
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/material/filter_chip_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';

import 'feedback_tester.dart';
import '../widgets/feedback_tester.dart';

/// Adds the basic requirements for a Chip.
Widget wrapForChip({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import 'dart:ui';
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter_test/flutter_test.dart';
import '../widgets/feedback_tester.dart';
import '../widgets/semantics_tester.dart';
import 'feedback_tester.dart';

void main() {
final ThemeData material3Theme = ThemeData(useMaterial3: true);
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/material/icon_button_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
import '../widgets/feedback_tester.dart';
import '../widgets/semantics_tester.dart';
import 'feedback_tester.dart';

class MockOnPressedFunction {
int called = 0;
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/material/ink_well_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/src/services/keyboard_key.g.dart';
import 'package:flutter_test/flutter_test.dart';
import '../widgets/feedback_tester.dart';
import '../widgets/semantics_tester.dart';
import 'feedback_tester.dart';

void main() {
testWidgets('InkWell gestures control test', (WidgetTester tester) async {
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/material/list_tile_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
import '../widgets/feedback_tester.dart';
import '../widgets/semantics_tester.dart';
import 'feedback_tester.dart';

class TestIcon extends StatefulWidget {
const TestIcon({ super.key });
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/material/popup_menu_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import 'package:flutter/rendering.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';

import '../widgets/feedback_tester.dart';
import '../widgets/semantics_tester.dart';
import 'feedback_tester.dart';

void main() {
testWidgets('Navigator.push works within a PopupMenuButton', (WidgetTester tester) async {
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/material/radio_list_tile_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';

import '../widgets/feedback_tester.dart';
import '../widgets/semantics_tester.dart';
import 'feedback_tester.dart';

Widget wrap({Widget? child}) {
return MediaQuery(
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/material/switch_list_tile_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter_test/flutter_test.dart';

import '../widgets/feedback_tester.dart';
import '../widgets/semantics_tester.dart';
import 'feedback_tester.dart';

Widget wrap({ required Widget child }) {
return MediaQuery(
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/material/tabs_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';

import '../widgets/feedback_tester.dart';
import '../widgets/semantics_tester.dart';
import 'feedback_tester.dart';
import 'tabs_utils.dart';

Widget boilerplate({
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/material/text_field_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ import 'package:flutter_test/flutter_test.dart';

import '../widgets/clipboard_utils.dart';
import '../widgets/editable_text_utils.dart';
import '../widgets/feedback_tester.dart';
import '../widgets/live_text_utils.dart';
import '../widgets/process_text_utils.dart';
import '../widgets/semantics_tester.dart';
import '../widgets/text_selection_toolbar_utils.dart';
import 'feedback_tester.dart';

void main() {
TestWidgetsFlutterBinding.ensureInitialized();
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/material/time_picker_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter_test/flutter_test.dart';

import '../widgets/feedback_tester.dart';
import '../widgets/semantics_tester.dart';
import 'feedback_tester.dart';

void main() {
const String okString = 'OK';
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/material/tooltip_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
import '../widgets/feedback_tester.dart';
import '../widgets/semantics_tester.dart';
import 'feedback_tester.dart';

const String tooltipText = 'TIP';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart';
import '../widgets/semantics_tester.dart';
import 'feedback_tester.dart';
import 'semantics_tester.dart';

void main () {
const Duration kWaitDuration = Duration(seconds: 1);
Expand Down Expand Up @@ -166,36 +166,34 @@ void main () {

group('Feedback on iOS', () {
testWidgets('forTap', (WidgetTester tester) async {
await tester.pumpWidget(Theme(
data: ThemeData(platform: TargetPlatform.iOS),
child: TestWidget(
await tester.pumpWidget(TestWidget(
tapHandler: (BuildContext context) {
return () => Feedback.forTap(context);
},
),
));
);

await tester.tap(find.text('X'));
await tester.pumpAndSettle(kWaitDuration);
expect(feedback.hapticCount, 0);
expect(feedback.clickSoundCount, 0);
});
},
variant: TargetPlatformVariant.only(TargetPlatform.iOS));

testWidgets('forLongPress', (WidgetTester tester) async {
await tester.pumpWidget(Theme(
data: ThemeData(platform: TargetPlatform.iOS),
child: TestWidget(
await tester.pumpWidget(TestWidget(
longPressHandler: (BuildContext context) {
return () => Feedback.forLongPress(context);
},
),
));
);

await tester.longPress(find.text('X'));
await tester.pumpAndSettle(kWaitDuration);
expect(feedback.hapticCount, 0);
expect(feedback.clickSoundCount, 0);
});
},
variant: TargetPlatformVariant.only(TargetPlatform.iOS));
});
}

Expand Down

0 comments on commit f7012db

Please sign in to comment.