Unverified Commit 4648f544 authored by Taha Tesser's avatar Taha Tesser Committed by GitHub

Fix chips `onDeleted` callback don't show the delete button when disabled (#137685)

fixes [Chips with `onDeleted` callback should show the delete button in the `disabled` state](https://github.com/flutter/flutter/issues/136638)

### 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> {

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.spaceEvenly,
          children: <Widget>[
            RawChip(
              avatar: const Icon(Icons.favorite_rounded),
              label: const Text('RawChip'),
              onSelected: null,
              isEnabled: false,
              onDeleted: () {},
            ),
            InputChip(
              avatar: const Icon(Icons.favorite_rounded),
              label: const Text('InputChip'),
              isEnabled: false,
              onPressed: null,
              onDeleted: () {},
            ),
            FilterChip(
              avatar: const Icon(Icons.favorite_rounded),
              label: const Text('FilterChip'),
              onSelected: null,
              onDeleted: () {},
            ),
          ],
        ),
      ),
    );
  }
}
```

</details>

| Before | After |
| --------------- | --------------- |
| <img src="https://github.com/flutter/flutter/assets/48603081/8bd458de-cfd2-44f0-a0dd-a8298938c61f" /> | <img src="https://github.com/flutter/flutter/assets/48603081/afca0684-b061-416b-b029-5316588c6888" /> |
parent 023e5add
...@@ -2064,26 +2064,39 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip ...@@ -2064,26 +2064,39 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip
} }
} }
final LayerHandle<OpacityLayer> _childOpacityLayerHandler = LayerHandle<OpacityLayer>(); final LayerHandle<OpacityLayer> _labelOpacityLayerHandler = LayerHandle<OpacityLayer>();
final LayerHandle<OpacityLayer> _deleteIconOpacityLayerHandler = LayerHandle<OpacityLayer>();
void _paintChild(PaintingContext context, Offset offset, RenderBox? child, bool? isEnabled) { void _paintChild(PaintingContext context, Offset offset, RenderBox? child, {required bool isDeleteIcon}) {
if (child == null) { if (child == null) {
_childOpacityLayerHandler.layer = null; _labelOpacityLayerHandler.layer = null;
_deleteIconOpacityLayerHandler.layer = null;
return; return;
} }
final int disabledColorAlpha = _disabledColor.alpha; final int disabledColorAlpha = _disabledColor.alpha;
if (!enableAnimation.isCompleted) { if (!enableAnimation.isCompleted) {
if (needsCompositing) { if (needsCompositing) {
_childOpacityLayerHandler.layer = context.pushOpacity( _labelOpacityLayerHandler.layer = context.pushOpacity(
offset, offset,
disabledColorAlpha, disabledColorAlpha,
(PaintingContext context, Offset offset) { (PaintingContext context, Offset offset) {
context.paintChild(child, _boxParentData(child).offset + offset); context.paintChild(child, _boxParentData(child).offset + offset);
}, },
oldLayer: _childOpacityLayerHandler.layer, oldLayer: _labelOpacityLayerHandler.layer,
); );
if (isDeleteIcon) {
_deleteIconOpacityLayerHandler.layer = context.pushOpacity(
offset,
disabledColorAlpha,
(PaintingContext context, Offset offset) {
context.paintChild(child, _boxParentData(child).offset + offset);
},
oldLayer: _deleteIconOpacityLayerHandler.layer,
);
}
} else { } else {
_childOpacityLayerHandler.layer = null; _labelOpacityLayerHandler.layer = null;
_deleteIconOpacityLayerHandler.layer = null;
final Rect childRect = _boxRect(child).shift(offset); final Rect childRect = _boxRect(child).shift(offset);
context.canvas.saveLayer(childRect.inflate(20.0), Paint()..color = _disabledColor); context.canvas.saveLayer(childRect.inflate(20.0), Paint()..color = _disabledColor);
context.paintChild(child, _boxParentData(child).offset + offset); context.paintChild(child, _boxParentData(child).offset + offset);
...@@ -2114,7 +2127,8 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip ...@@ -2114,7 +2127,8 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip
@override @override
void dispose() { void dispose() {
_childOpacityLayerHandler.layer = null; _labelOpacityLayerHandler.layer = null;
_deleteIconOpacityLayerHandler.layer = null;
_avatarOpacityLayerHandler.layer = null; _avatarOpacityLayerHandler.layer = null;
super.dispose(); super.dispose();
} }
...@@ -2123,9 +2137,9 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip ...@@ -2123,9 +2137,9 @@ class _RenderChip extends RenderBox with SlottedContainerRenderObjectMixin<_Chip
void paint(PaintingContext context, Offset offset) { void paint(PaintingContext context, Offset offset) {
_paintAvatar(context, offset); _paintAvatar(context, offset);
if (deleteIconShowing) { if (deleteIconShowing) {
_paintChild(context, offset, deleteIcon, isEnabled); _paintChild(context, offset, deleteIcon, isDeleteIcon: true);
} }
_paintChild(context, offset, label, isEnabled); _paintChild(context, offset, label, isDeleteIcon: false);
} }
// Set this to true to have outlines of the tap targets drawn over // Set this to true to have outlines of the tap targets drawn over
......
...@@ -2,6 +2,11 @@ ...@@ -2,6 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// This file is run as part of a reduced test set in CI on Mac and Windows
// machines.
@Tags(<String>['reduced-test-set'])
library;
import 'package:flutter/gestures.dart'; import 'package:flutter/gestures.dart';
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart'; import 'package:flutter/rendering.dart';
...@@ -3653,6 +3658,21 @@ void main() { ...@@ -3653,6 +3658,21 @@ void main() {
expect(tester.takeException(), isNull); expect(tester.takeException(), isNull);
}); });
testWidgetsWithLeakTracking('Delete button is visible RawChip is disabled', (WidgetTester tester) async {
await tester.pumpWidget(
wrapForChip(
child: RawChip(
isEnabled: false,
label: const Text('Label'),
onDeleted: () { },
)
),
);
// Delete button should be visible.
expectLater(find.byType(RawChip), matchesGoldenFile('raw_chip.disabled.delete_button.png'));
});
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
......
...@@ -2,6 +2,11 @@ ...@@ -2,6 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// This file is run as part of a reduced test set in CI on Mac and Windows
// machines.
@Tags(<String>['reduced-test-set'])
library;
import 'package:flutter/foundation.dart'; import 'package:flutter/foundation.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';
...@@ -919,4 +924,19 @@ void main() { ...@@ -919,4 +924,19 @@ void main() {
feedback.dispose(); feedback.dispose();
}); });
testWidgetsWithLeakTracking('Delete button is visible FilterChip is disabled', (WidgetTester tester) async {
await tester.pumpWidget(
wrapForChip(
child: FilterChip(
label: const Text('Label'),
onSelected: null,
onDeleted: () { },
)
),
);
// Delete button should be visible.
expectLater(find.byType(RawChip), matchesGoldenFile('filter_chip.disabled.delete_button.png'));
});
} }
...@@ -2,6 +2,11 @@ ...@@ -2,6 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// This file is run as part of a reduced test set in CI on Mac and Windows
// machines.
@Tags(<String>['reduced-test-set'])
library;
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart'; import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';
...@@ -411,4 +416,19 @@ void main() { ...@@ -411,4 +416,19 @@ void main() {
expect(getIconData(tester).color, const Color(0xff00ff00)); expect(getIconData(tester).color, const Color(0xff00ff00));
}); });
testWidgetsWithLeakTracking('Delete button is visible InputChip is disabled', (WidgetTester tester) async {
await tester.pumpWidget(
wrapForChip(
child: InputChip(
isEnabled: false,
label: const Text('Label'),
onDeleted: () { },
)
),
);
// Delete button should be visible.
expectLater(find.byType(RawChip), matchesGoldenFile('input_chip.disabled.delete_button.png'));
});
} }
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