Skip to content

Fix SegmentedButton clipping when drawing segments #149739

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 7, 2024

Conversation

TahaTesser
Copy link
Member

@TahaTesser TahaTesser commented Jun 5, 2024

fixes SegmentedButton doesn't clip properly when one of the segments has ColorFiltered

Code sample

expand to view the code sample
import 'package:flutter/material.dart';

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

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

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        body: Center(
          child: SegmentedButton<int>(
            segments: const <ButtonSegment<int>>[
              ButtonSegment<int>(
                value: 0,
                label: ColorFiltered(
                  colorFilter:
                      ColorFilter.mode(Colors.amber, BlendMode.colorBurn),
                  child: Text('Option 1'),
                ),
              ),
              ButtonSegment<int>(
                value: 1,
                label: Text('Option 2'),
              ),
            ],
            onSelectionChanged: (Set<int> selected) {},
            selected: const <int>{0},
          ),
        ),
      ),
    );
  }
}

Before

before

After

after

Pre-launch Checklist

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

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Jun 5, 2024
@TahaTesser TahaTesser force-pushed the fix_button_segments_clipping branch from ea8d2e5 to 38aa644 Compare June 5, 2024 13:50
@TahaTesser TahaTesser changed the title Fix SegmentedButton clipping drawing segments Fix SegmentedButton clipping when drawing segments Jun 5, 2024
@TahaTesser TahaTesser marked this pull request as ready for review June 5, 2024 13:50
@TahaTesser TahaTesser requested a review from QuncCccccc June 5, 2024 13:50
Copy link
Contributor

@QuncCccccc QuncCccccc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also help try other platforms? It seems the vertical line is still not clipped on mobile devices but correctly clipped on maxOS and web🧐 The splash effect when we press the button is clipped correctly with this change though:)
Screenshot 2024-06-05 at 5 04 33 PM

@TahaTesser
Copy link
Member Author

TahaTesser commented Jun 6, 2024

Could you also help try other platforms? It seems the vertical line is still not clipped on mobile devices but correctly clipped on maxOS and web🧐 The splash effect when we press the button is clipped correctly with this change though:)

Thank you so much! I should've tested this on mobile.

The reason divider is out the border because previously segmented button was using childRect to clip the canvas and paint the child. In my fix, I'm using border path to clip the canvas.

The divider bug was due to it is still using the childRect to position, I fixed this by updating divider offset and added a new test to catch this bug in the future.

Preview

@TahaTesser TahaTesser requested a review from QuncCccccc June 6, 2024 11:22
Copy link
Contributor

@QuncCccccc QuncCccccc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The divider bug was due to it is still using the childRect to position, I fixed this by updating divider offset and added a new test to catch this bug in the future.

Ah, this makes sense to me. Thanks for the explanation and the test! LGTM:)

@TahaTesser TahaTesser force-pushed the fix_button_segments_clipping branch from 228b549 to 5e62da5 Compare June 7, 2024 07:17
@TahaTesser
Copy link
Member Author

@QuncCccccc
This is failing Google Testing, it might be legit. Can you please take a look?

@QuncCccccc
Copy link
Contributor

QuncCccccc commented Jun 7, 2024

@QuncCccccc This is failing Google Testing, it might be legit. Can you please take a look?

Checking!

Update: I can't really see the difference based on the change, so I'm going to manually update Google testing status:)

@QuncCccccc QuncCccccc added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 7, 2024
@auto-submit auto-submit bot merged commit fc19ecf into flutter:master Jun 7, 2024
72 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2024
@QuncCccccc
Copy link
Contributor

Reason for revert: the newly-added unit test might cause the red tree

@QuncCccccc QuncCccccc added the revert Autorevert PR (with "Reason for revert:" comment) label Jun 7, 2024
auto-submit bot pushed a commit that referenced this pull request Jun 7, 2024
@auto-submit auto-submit bot removed the revert Autorevert PR (with "Reason for revert:" comment) label Jun 7, 2024
auto-submit bot added a commit that referenced this pull request Jun 7, 2024
…" (#149927)

Reverts: #149739
Initiated by: QuncCccccc
Reason for reverting: the newly-added unit test might cause the red tree
Original PR Author: TahaTesser

Reviewed By: {QuncCccccc}

This change reverts the following previous change:
fixes [`SegmentedButton` doesn't clip properly when one of the segments has `ColorFiltered`](#144990)

### 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 MaterialApp(
      home: Scaffold(
        body: Center(
          child: SegmentedButton<int>(
            segments: const <ButtonSegment<int>>[
              ButtonSegment<int>(
                value: 0,
                label: ColorFiltered(
                  colorFilter:
                      ColorFilter.mode(Colors.amber, BlendMode.colorBurn),
                  child: Text('Option 1'),
                ),
              ),
              ButtonSegment<int>(
                value: 1,
                label: Text('Option 2'),
              ),
            ],
            onSelectionChanged: (Set<int> selected) {},
            selected: const <int>{0},
          ),
        ),
      ),
    );
  }
}
```

</details>

### Before

![before](https://github.com/flutter/flutter/assets/48603081/b402fc51-d575-4208-8a71-f798ef2b2bf5)

### After

![after](https://github.com/flutter/flutter/assets/48603081/77a5cac2-ecef-4381-a043-dbb5c91407ec)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jun 7, 2024
Manual roll Flutter from 4608a89 to fc19ecf (38 revisions)

Manual roll requested by [email protected]

flutter/flutter@4608a89...fc19ecf

2024-06-07 [email protected] Fix `SegmentedButton` clipping when drawing segments (flutter/flutter#149739)
2024-06-07 [email protected] Shift consistently passing tests on mokey to prod (flutter/flutter#149902)
2024-06-07 [email protected] Prepare images for tests individually to enable clean up of cache. (flutter/flutter#149693)
2024-06-07 [email protected] Roll Packages from 586faa6 to 8a2c4e4 (8 revisions) (flutter/flutter#149900)
2024-06-07 [email protected] Roll Flutter Engine from 98e9393bf2e9 to 1cdbebee1901 (4 revisions) (flutter/flutter#149899)
2024-06-07 [email protected] Roll Flutter Engine from 087feab9922b to 98e9393bf2e9 (1 revision) (flutter/flutter#149875)
2024-06-07 [email protected] Refactor `widget_inspector_test.dart` (flutter/flutter#149850)
2024-06-07 [email protected] Add test for dropdown_menu.1.dart (flutter/flutter#149547)
2024-06-07 [email protected] Add test for standard_fab_location.0.dart (flutter/flutter#149225)
2024-06-07 [email protected] [native_assets] Fix framework name deduplication (flutter/flutter#149761)
2024-06-07 [email protected] Roll Flutter Engine from 47b15dfba1da to 087feab9922b (1 revision) (flutter/flutter#149871)
2024-06-07 [email protected] Roll Flutter Engine from 5d5e0ee9102c to 47b15dfba1da (2 revisions) (flutter/flutter#149870)
2024-06-07 [email protected] Roll Flutter Engine from fea8e51b0280 to 5d5e0ee9102c (2 revisions) (flutter/flutter#149867)
2024-06-07 [email protected] Roll Flutter Engine from 5a0a49939c64 to fea8e51b0280 (2 revisions) (flutter/flutter#149865)
2024-06-07 [email protected] Roll pub packages (flutter/flutter#149864)
2024-06-06 [email protected] Roll Flutter Engine from bf1a4877da0b to 5a0a49939c64 (1 revision) (flutter/flutter#149861)
2024-06-06 [email protected] Remove some vestigial /*!*/ comments (flutter/flutter#149361)
2024-06-06 [email protected] Attempt to fix some web test flakiness. (flutter/flutter#149828)
2024-06-06 [email protected] Remove abi key permanently from golden file testing (flutter/flutter#149858)
2024-06-06 [email protected] [CupertinoActionSheet] Add sliding tap gesture (flutter/flutter#149471)
2024-06-06 [email protected] Roll pub packages (flutter/flutter#149852)
2024-06-06 [email protected] SliverResizingHeader (flutter/flutter#143325)
2024-06-06 [email protected] Run tests on macOS 13 or 14 (flutter/flutter#149827)
2024-06-06 [email protected] Fix leaky test. (flutter/flutter#149823)
2024-06-06 [email protected] Roll Flutter Engine from 31804a222f97 to bf1a4877da0b (3 revisions) (flutter/flutter#149848)
2024-06-06 [email protected] Add support for setting the heading level for web semantics (#97894) (flutter/flutter#125771)
2024-06-06 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Introduce `ChipAnimationStyle` to override default chips animations durations (#149245)" (flutter/flutter#149847)
2024-06-06 [email protected] 3.22.2 release notes (flutter/flutter#149818)
2024-06-06 [email protected] Roll Flutter Engine from 25bcbb888939 to 31804a222f97 (3 revisions) (flutter/flutter#149837)
2024-06-06 [email protected] Introduce `ChipAnimationStyle` to override default chips animations durations (flutter/flutter#149245)
2024-06-06 [email protected] Roll Flutter Engine from f5a9690c6061 to 25bcbb888939 (1 revision) (flutter/flutter#149835)
2024-06-06 [email protected] Fix leaky test. (flutter/flutter#149822)
2024-06-06 [email protected] Unpin archive package from tool, update to version without security advisories (flutter/flutter#149430)
2024-06-06 [email protected] Roll pub packages (flutter/flutter#149630)
2024-06-06 [email protected] Roll Flutter Engine from d9684a37af06 to f5a9690c6061 (7 revisions) (flutter/flutter#149832)
2024-06-06 [email protected] Roll Packages from 11e192a to 586faa6 (13 revisions) (flutter/flutter#149820)
2024-06-06 [email protected] Fix some links in the "Handling breaking change" section (flutter/flutter#149821)
2024-06-06 [email protected] Roll Flutter Engine from 92d0cd9370f7 to d9684a37af06 (1 revision) (flutter/flutter#149793)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.
...
@TahaTesser TahaTesser deleted the fix_button_segments_clipping branch June 12, 2024 08:46
TahaTesser added a commit to TahaTesser/flutter that referenced this pull request Jun 12, 2024
fixes [`SegmentedButton` doesn't clip properly when one of the segments has `ColorFiltered`](flutter#144990)

### 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 MaterialApp(
      home: Scaffold(
        body: Center(
          child: SegmentedButton<int>(
            segments: const <ButtonSegment<int>>[
              ButtonSegment<int>(
                value: 0,
                label: ColorFiltered(
                  colorFilter:
                      ColorFilter.mode(Colors.amber, BlendMode.colorBurn),
                  child: Text('Option 1'),
                ),
              ),
              ButtonSegment<int>(
                value: 1,
                label: Text('Option 2'),
              ),
            ],
            onSelectionChanged: (Set<int> selected) {},
            selected: const <int>{0},
          ),
        ),
      ),
    );
  }
}
```

</details>

### Before

![before](https://github.com/flutter/flutter/assets/48603081/b402fc51-d575-4208-8a71-f798ef2b2bf5)

### After

![after](https://github.com/flutter/flutter/assets/48603081/77a5cac2-ecef-4381-a043-dbb5c91407ec)
auto-submit bot pushed a commit that referenced this pull request Jun 12, 2024
… (#150090)

Relands #149739 which was reverted due to #149851

--- 

fixes [`SegmentedButton` doesn't clip properly when one of the segments has `ColorFiltered`](#144990)

### 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 MaterialApp(
      home: Scaffold(
        body: Center(
          child: SegmentedButton<int>(
            segments: const <ButtonSegment<int>>[
              ButtonSegment<int>(
                value: 0,
                label: ColorFiltered(
                  colorFilter:
                      ColorFilter.mode(Colors.amber, BlendMode.colorBurn),
                  child: Text('Option 1'),
                ),
              ),
              ButtonSegment<int>(
                value: 1,
                label: Text('Option 2'),
              ),
            ],
            onSelectionChanged: (Set<int> selected) {},
            selected: const <int>{0},
          ),
        ),
      ),
    );
  }
}
```

</details>

### Before

![before](https://github.com/flutter/flutter/assets/48603081/b402fc51-d575-4208-8a71-f798ef2b2bf5)

### After

![after](https://github.com/flutter/flutter/assets/48603081/77a5cac2-ecef-4381-a043-dbb5c91407ec)

*Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.*

*List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.*

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
arc-yong pushed a commit to Arctuition/packages-arc that referenced this pull request Jun 14, 2024
)

Manual roll Flutter from 4608a89 to fc19ecf (38 revisions)

Manual roll requested by [email protected]

flutter/flutter@4608a89...fc19ecf

2024-06-07 [email protected] Fix `SegmentedButton` clipping when drawing segments (flutter/flutter#149739)
2024-06-07 [email protected] Shift consistently passing tests on mokey to prod (flutter/flutter#149902)
2024-06-07 [email protected] Prepare images for tests individually to enable clean up of cache. (flutter/flutter#149693)
2024-06-07 [email protected] Roll Packages from 586faa6 to 8a2c4e4 (8 revisions) (flutter/flutter#149900)
2024-06-07 [email protected] Roll Flutter Engine from 98e9393bf2e9 to 1cdbebee1901 (4 revisions) (flutter/flutter#149899)
2024-06-07 [email protected] Roll Flutter Engine from 087feab9922b to 98e9393bf2e9 (1 revision) (flutter/flutter#149875)
2024-06-07 [email protected] Refactor `widget_inspector_test.dart` (flutter/flutter#149850)
2024-06-07 [email protected] Add test for dropdown_menu.1.dart (flutter/flutter#149547)
2024-06-07 [email protected] Add test for standard_fab_location.0.dart (flutter/flutter#149225)
2024-06-07 [email protected] [native_assets] Fix framework name deduplication (flutter/flutter#149761)
2024-06-07 [email protected] Roll Flutter Engine from 47b15dfba1da to 087feab9922b (1 revision) (flutter/flutter#149871)
2024-06-07 [email protected] Roll Flutter Engine from 5d5e0ee9102c to 47b15dfba1da (2 revisions) (flutter/flutter#149870)
2024-06-07 [email protected] Roll Flutter Engine from fea8e51b0280 to 5d5e0ee9102c (2 revisions) (flutter/flutter#149867)
2024-06-07 [email protected] Roll Flutter Engine from 5a0a49939c64 to fea8e51b0280 (2 revisions) (flutter/flutter#149865)
2024-06-07 [email protected] Roll pub packages (flutter/flutter#149864)
2024-06-06 [email protected] Roll Flutter Engine from bf1a4877da0b to 5a0a49939c64 (1 revision) (flutter/flutter#149861)
2024-06-06 [email protected] Remove some vestigial /*!*/ comments (flutter/flutter#149361)
2024-06-06 [email protected] Attempt to fix some web test flakiness. (flutter/flutter#149828)
2024-06-06 [email protected] Remove abi key permanently from golden file testing (flutter/flutter#149858)
2024-06-06 [email protected] [CupertinoActionSheet] Add sliding tap gesture (flutter/flutter#149471)
2024-06-06 [email protected] Roll pub packages (flutter/flutter#149852)
2024-06-06 [email protected] SliverResizingHeader (flutter/flutter#143325)
2024-06-06 [email protected] Run tests on macOS 13 or 14 (flutter/flutter#149827)
2024-06-06 [email protected] Fix leaky test. (flutter/flutter#149823)
2024-06-06 [email protected] Roll Flutter Engine from 31804a222f97 to bf1a4877da0b (3 revisions) (flutter/flutter#149848)
2024-06-06 [email protected] Add support for setting the heading level for web semantics (#97894) (flutter/flutter#125771)
2024-06-06 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Introduce `ChipAnimationStyle` to override default chips animations durations (#149245)" (flutter/flutter#149847)
2024-06-06 [email protected] 3.22.2 release notes (flutter/flutter#149818)
2024-06-06 [email protected] Roll Flutter Engine from 25bcbb888939 to 31804a222f97 (3 revisions) (flutter/flutter#149837)
2024-06-06 [email protected] Introduce `ChipAnimationStyle` to override default chips animations durations (flutter/flutter#149245)
2024-06-06 [email protected] Roll Flutter Engine from f5a9690c6061 to 25bcbb888939 (1 revision) (flutter/flutter#149835)
2024-06-06 [email protected] Fix leaky test. (flutter/flutter#149822)
2024-06-06 [email protected] Unpin archive package from tool, update to version without security advisories (flutter/flutter#149430)
2024-06-06 [email protected] Roll pub packages (flutter/flutter#149630)
2024-06-06 [email protected] Roll Flutter Engine from d9684a37af06 to f5a9690c6061 (7 revisions) (flutter/flutter#149832)
2024-06-06 [email protected] Roll Packages from 11e192a to 586faa6 (13 revisions) (flutter/flutter#149820)
2024-06-06 [email protected] Fix some links in the "Handling breaking change" section (flutter/flutter#149821)
2024-06-06 [email protected] Roll Flutter Engine from 92d0cd9370f7 to d9684a37af06 (1 revision) (flutter/flutter#149793)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.
...
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request Jun 14, 2024
…r#149739) (flutter#150090)

Relands flutter#149739 which was reverted due to flutter#149851

--- 

fixes [`SegmentedButton` doesn't clip properly when one of the segments has `ColorFiltered`](flutter#144990)

### 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 MaterialApp(
      home: Scaffold(
        body: Center(
          child: SegmentedButton<int>(
            segments: const <ButtonSegment<int>>[
              ButtonSegment<int>(
                value: 0,
                label: ColorFiltered(
                  colorFilter:
                      ColorFilter.mode(Colors.amber, BlendMode.colorBurn),
                  child: Text('Option 1'),
                ),
              ),
              ButtonSegment<int>(
                value: 1,
                label: Text('Option 2'),
              ),
            ],
            onSelectionChanged: (Set<int> selected) {},
            selected: const <int>{0},
          ),
        ),
      ),
    );
  }
}
```

</details>

### Before

![before](https://github.com/flutter/flutter/assets/48603081/b402fc51-d575-4208-8a71-f798ef2b2bf5)

### After

![after](https://github.com/flutter/flutter/assets/48603081/77a5cac2-ecef-4381-a043-dbb5c91407ec)

*Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.*

*List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.*

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request Jun 14, 2024
…r#149739) (flutter#150090)

Relands flutter#149739 which was reverted due to flutter#149851

--- 

fixes [`SegmentedButton` doesn't clip properly when one of the segments has `ColorFiltered`](flutter#144990)

### 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 MaterialApp(
      home: Scaffold(
        body: Center(
          child: SegmentedButton<int>(
            segments: const <ButtonSegment<int>>[
              ButtonSegment<int>(
                value: 0,
                label: ColorFiltered(
                  colorFilter:
                      ColorFilter.mode(Colors.amber, BlendMode.colorBurn),
                  child: Text('Option 1'),
                ),
              ),
              ButtonSegment<int>(
                value: 1,
                label: Text('Option 2'),
              ),
            ],
            onSelectionChanged: (Set<int> selected) {},
            selected: const <int>{0},
          ),
        ),
      ),
    );
  }
}
```

</details>

### Before

![before](https://github.com/flutter/flutter/assets/48603081/b402fc51-d575-4208-8a71-f798ef2b2bf5)

### After

![after](https://github.com/flutter/flutter/assets/48603081/77a5cac2-ecef-4381-a043-dbb5c91407ec)

*Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.*

*List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.*

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SegmentedButton doesn't clip properly when one of the segments has ColorFiltered
2 participants