Unverified Commit 788614d1 authored by Taha Tesser's avatar Taha Tesser Committed by GitHub

Fix "Delete" tooltip is shown disabled on chips with `onDeleted` callback (#141770)

fixes [Disabled chips with `onDeleted` callback shows "Delete" tooltip on hover](https://github.com/flutter/flutter/issues/141336)

### Code sample

<details>
<summary>expand to view the code sample</summary> 

```dart
import 'package:flutter/material.dart';

void main() => runApp(const MyApp());

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return const MaterialApp(
      debugShowCheckedModeBanner: false,
      home: Example(),
    );
  }
}

class Example extends StatefulWidget {
  const Example({super.key});

  @override
  State<Example> createState() => _ExampleState();
}

class _ExampleState extends State<Example> {
  bool _isEnable = false;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.spaceEvenly,
          children: <Widget>[
            RawChip(
              label: const Text('RawChip'),
              onPressed: () {},
              isEnabled: _isEnable,
              onDeleted: () {},
            ),
            FilterChip(
              label: const Text('FilterChip'),
              selected: false,
              onSelected: _isEnable ? (bool value) {} : null,
              onDeleted: () {},
            ),
            InputChip(
              label: const Text('InputChip'),
              isEnabled: _isEnable,
              onDeleted: () {},
            ),
          ],
        ),
      ),
      floatingActionButton: FloatingActionButton.extended(
        onPressed: () {
          setState(() {
            _isEnable = !_isEnable;
          });
        },
        label: Text(_isEnable ? 'Disable' : 'Enable'),
      ),
    );
  }
}
```

</details>

### Preview

| Before | After |
| --------------- | --------------- |
| <img src="https://github.com/flutter/flutter/assets/48603081/f80ae5f7-0a6d-4041-ade3-cbc2b5c78188" height="450" /> | <img src="https://github.com/flutter/flutter/assets/48603081/04e62854-e3f1-4b65-9753-183d288f3cfe" height="450" /> |
parent 6f7aed59
...@@ -280,6 +280,8 @@ abstract interface class DeletableChipAttributes { ...@@ -280,6 +280,8 @@ abstract interface class DeletableChipAttributes {
/// ///
/// If null, the default [MaterialLocalizations.deleteButtonTooltip] will be /// If null, the default [MaterialLocalizations.deleteButtonTooltip] will be
/// used. /// used.
///
/// If the chip is disabled, the delete button tooltip will not be shown.
String? get deleteButtonTooltipMessage; String? get deleteButtonTooltipMessage;
} }
...@@ -1128,7 +1130,7 @@ class _RawChipState extends State<RawChip> with MaterialStateMixin, TickerProvid ...@@ -1128,7 +1130,7 @@ class _RawChipState extends State<RawChip> with MaterialStateMixin, TickerProvid
child: _wrapWithTooltip( child: _wrapWithTooltip(
tooltip: widget.deleteButtonTooltipMessage tooltip: widget.deleteButtonTooltipMessage
?? MaterialLocalizations.of(context).deleteButtonTooltip, ?? MaterialLocalizations.of(context).deleteButtonTooltip,
enabled: widget.onDeleted != null, enabled: widget.isEnabled && widget.onDeleted != null,
child: InkWell( child: InkWell(
// Radius should be slightly less than the full size of the chip. // Radius should be slightly less than the full size of the chip.
radius: (_kChipHeight + (widget.padding?.vertical ?? 0.0)) * .45, radius: (_kChipHeight + (widget.padding?.vertical ?? 0.0)) * .45,
......
...@@ -5112,7 +5112,7 @@ void main() { ...@@ -5112,7 +5112,7 @@ void main() {
expect(tester.takeException(), isNull); expect(tester.takeException(), isNull);
}); });
testWidgets('Delete button is visible RawChip is disabled', (WidgetTester tester) async { testWidgets('Delete button is visible on disabled RawChip', (WidgetTester tester) async {
await tester.pumpWidget( await tester.pumpWidget(
wrapForChip( wrapForChip(
child: RawChip( child: RawChip(
...@@ -5127,6 +5127,36 @@ void main() { ...@@ -5127,6 +5127,36 @@ void main() {
expectLater(find.byType(RawChip), matchesGoldenFile('raw_chip.disabled.delete_button.png')); expectLater(find.byType(RawChip), matchesGoldenFile('raw_chip.disabled.delete_button.png'));
}); });
testWidgets('Delete button tooltip is not shown on disabled RawChip', (WidgetTester tester) async {
Widget buildChip({ bool enabled = true }) {
return wrapForChip(
child: RawChip(
isEnabled: enabled,
label: const Text('Label'),
onDeleted: () { },
)
);
}
// Test enabled chip.
await tester.pumpWidget(buildChip());
final Offset deleteButtonLocation = tester.getCenter(find.byType(Icon));
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.moveTo(deleteButtonLocation);
await tester.pump();
// Delete button tooltip should be visible.
expect(findTooltipContainer('Delete'), findsOneWidget);
// Test disabled chip.
await tester.pumpWidget(buildChip(enabled: false));
await tester.pump();
// Delete button tooltip should not be visible.
expect(findTooltipContainer('Delete'), findsNothing);
});
group('Material 2', () { group('Material 2', () {
// These tests are only relevant for Material 2. Once Material 2 // These tests are only relevant for Material 2. Once Material 2
// support is deprecated and the APIs are removed, these tests // support is deprecated and the APIs are removed, these tests
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
library; library;
import 'package:flutter/foundation.dart'; import 'package:flutter/foundation.dart';
import 'package:flutter/gestures.dart';
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
...@@ -130,6 +131,14 @@ DefaultTextStyle getLabelStyle(WidgetTester tester, String labelText) { ...@@ -130,6 +131,14 @@ DefaultTextStyle getLabelStyle(WidgetTester tester, String labelText) {
); );
} }
// Finds any container of a tooltip.
Finder findTooltipContainer(String tooltipText) {
return find.ancestor(
of: find.text(tooltipText),
matching: find.byType(Container),
);
}
void main() { void main() {
testWidgets('Material2 - FilterChip defaults', (WidgetTester tester) async { testWidgets('Material2 - FilterChip defaults', (WidgetTester tester) async {
final ThemeData theme = ThemeData(useMaterial3: false); final ThemeData theme = ThemeData(useMaterial3: false);
...@@ -1040,7 +1049,7 @@ void main() { ...@@ -1040,7 +1049,7 @@ void main() {
feedback.dispose(); feedback.dispose();
}); });
testWidgets('Delete button is visible FilterChip is disabled', (WidgetTester tester) async { testWidgets('Delete button is visible on disabled FilterChip', (WidgetTester tester) async {
await tester.pumpWidget( await tester.pumpWidget(
wrapForChip( wrapForChip(
child: FilterChip( child: FilterChip(
...@@ -1054,4 +1063,34 @@ void main() { ...@@ -1054,4 +1063,34 @@ void main() {
// Delete button should be visible. // Delete button should be visible.
expectLater(find.byType(RawChip), matchesGoldenFile('filter_chip.disabled.delete_button.png')); expectLater(find.byType(RawChip), matchesGoldenFile('filter_chip.disabled.delete_button.png'));
}); });
testWidgets('Delete button tooltip is not shown on disabled FilterChip', (WidgetTester tester) async {
Widget buildChip({ bool enabled = true }) {
return wrapForChip(
child: FilterChip(
onSelected: enabled ? (bool value) { } : null,
label: const Text('Label'),
onDeleted: () { },
)
);
}
// Test enabled chip.
await tester.pumpWidget(buildChip());
final Offset deleteButtonLocation = tester.getCenter(find.byType(Icon));
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.moveTo(deleteButtonLocation);
await tester.pump();
// Delete button tooltip should be visible.
expect(findTooltipContainer('Delete'), findsOneWidget);
// Test disabled chip.
await tester.pumpWidget(buildChip(enabled: false));
await tester.pump();
// Delete button tooltip should not be visible.
expect(findTooltipContainer('Delete'), findsNothing);
});
} }
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
@Tags(<String>['reduced-test-set']) @Tags(<String>['reduced-test-set'])
library; library;
import 'package:flutter/gestures.dart';
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
...@@ -113,6 +114,14 @@ void checkChipMaterialClipBehavior(WidgetTester tester, Clip clipBehavior) { ...@@ -113,6 +114,14 @@ void checkChipMaterialClipBehavior(WidgetTester tester, Clip clipBehavior) {
expect(materials.last.clipBehavior, clipBehavior); expect(materials.last.clipBehavior, clipBehavior);
} }
// Finds any container of a tooltip.
Finder findTooltipContainer(String tooltipText) {
return find.ancestor(
of: find.text(tooltipText),
matching: find.byType(Container),
);
}
void main() { void main() {
testWidgets('InputChip.color resolves material states', (WidgetTester tester) async { testWidgets('InputChip.color resolves material states', (WidgetTester tester) async {
const Color disabledSelectedColor = Color(0xffffff00); const Color disabledSelectedColor = Color(0xffffff00);
...@@ -417,7 +426,7 @@ void main() { ...@@ -417,7 +426,7 @@ void main() {
expect(getIconData(tester).color, const Color(0xff00ff00)); expect(getIconData(tester).color, const Color(0xff00ff00));
}); });
testWidgets('Delete button is visible InputChip is disabled', (WidgetTester tester) async { testWidgets('Delete button is visible on disabled InputChip', (WidgetTester tester) async {
await tester.pumpWidget( await tester.pumpWidget(
wrapForChip( wrapForChip(
child: InputChip( child: InputChip(
...@@ -431,4 +440,34 @@ void main() { ...@@ -431,4 +440,34 @@ void main() {
// Delete button should be visible. // Delete button should be visible.
expectLater(find.byType(RawChip), matchesGoldenFile('input_chip.disabled.delete_button.png')); expectLater(find.byType(RawChip), matchesGoldenFile('input_chip.disabled.delete_button.png'));
}); });
testWidgets('Delete button tooltip is not shown on disabled InputChip', (WidgetTester tester) async {
Widget buildChip({ bool enabled = true }) {
return wrapForChip(
child: InputChip(
isEnabled: enabled,
label: const Text('Label'),
onDeleted: () { },
)
);
}
// Test enabled chip.
await tester.pumpWidget(buildChip());
final Offset deleteButtonLocation = tester.getCenter(find.byType(Icon));
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.moveTo(deleteButtonLocation);
await tester.pump();
// Delete button tooltip should be visible.
expect(findTooltipContainer('Delete'), findsOneWidget);
// Test disabled chip.
await tester.pumpWidget(buildChip(enabled: false));
await tester.pump();
// Delete button tooltip should not be visible.
expect(findTooltipContainer('Delete'), findsNothing);
});
} }
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