Skip to content

Conversation

@AcarFurkan
Copy link
Contributor

@AcarFurkan AcarFurkan commented Jun 20, 2023

Style from has been added to the segment button, so there is no longer any need to the button style from the beginning. secondly, height correction was made to set the height. Third, the expanded property has been added to take the width of the whole page.

simulator_screenshot_E95B01E8-4D02-4D4D-830A-8B58F496BBB3
simulator_screenshot_AA44B70B-7024-4372-AE1D-D7416AC99E89

issues
#121493 #123308

If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].

Pre-launch Checklist

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

Style from has been added to the segment button, so there is no longer any need to the button style from the beginning. secondly, height correction was made to set the height. Third, the expanded property has been added to take the width of the whole page.
@google-cla
Copy link

google-cla bot commented Jun 20, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Jun 20, 2023
@TahaTesser TahaTesser self-requested a review June 21, 2023 10:44
@TahaTesser
Copy link
Member

TahaTesser commented Jun 21, 2023

HI @AcarFurkan
Thanks for working on this.

This will need to be split into multiple PRs with separate tests and detailed explanations for the fix or new feature.

Please read the style guide on how to format the code and avoid using Dart format.

Make sure to not remove any public APIs as it will break the widget for the users.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@flutter-dashboard
Copy link

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@Hixie
Copy link
Contributor

Hixie commented Aug 29, 2023

@AcarFurkan Thanks for your contribution. @TahaTesser's review comments above are quite accurate though, we can't just arbitrarily make removals from the APIs, as it will break people using them. Also it seems like there are a lot of failing tests.

I'm going to close this PR since it cannot land in its current state, but if you are interested in pursuing this further I recommend filing a new PR with a patch that passes the tests and that does the refactor in a non-breaking way. Thanks!

@Hixie Hixie closed this Aug 29, 2023
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie or stuartmorgan on the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

auto-submit bot pushed a commit that referenced this pull request Jan 3, 2024
fixes #138289

---

SegmentedButtom.styleFrom has been added to the segment button, so there is no longer any need to the button style from the beginning. It works like ElevatedButton.styleFrom only I added selectedForegroundColor, selectedBackgroundColor. In this way, the user will be able to change the color first without checking the MaterialState states. I added tests of the same controls.

#129215 I opened this problem myself, but I was rejected because I handled too many items in a PR. For now, I wrote a structure that only handles MaterialStates instead of users.

old (still avaliable)
<img width="626" alt="image" src="https://github.com/flutter/flutter/assets/65075121/9446b13b-c355-4d20-bda2-c47a23d42d4f">

new (just an option for developer)
<img width="483" alt="image" src="https://github.com/flutter/flutter/assets/65075121/0a645257-4c83-4029-9484-bd746c02265f">

### Code sample

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

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

/// Flutter code sample for [SegmentedButton].

void main() {
  runApp(const SegmentedButtonApp());
}

enum Calendar { day, week, month, year }

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

  @OverRide
  State<SegmentedButtonApp> createState() => _SegmentedButtonAppState();
}

class _SegmentedButtonAppState extends State<SegmentedButtonApp> {
  Calendar calendarView = Calendar.day;

  @OverRide
  Widget build(BuildContext context) {
    return MaterialApp(
      theme: ThemeData(useMaterial3: true),
      home: Scaffold(
        body: Center(
          child: SegmentedButton<Calendar>(
            style: SegmentedButton.styleFrom(
              foregroundColor: Colors.amber,
              visualDensity: VisualDensity.comfortable,
            ),
            // style: const ButtonStyle(
            //   foregroundColor: MaterialStatePropertyAll<Color>(Colors.deepPurple),
            //   visualDensity: VisualDensity.comfortable,
            // ),
            segments: const <ButtonSegment<Calendar>>[
              ButtonSegment<Calendar>(
                  value: Calendar.day,
                  label: Text('Day'),
                  icon: Icon(Icons.calendar_view_day)),
              ButtonSegment<Calendar>(
                  value: Calendar.week,
                  label: Text('Week'),
                  icon: Icon(Icons.calendar_view_week)),
              ButtonSegment<Calendar>(
                  value: Calendar.month,
                  label: Text('Month'),
                  icon: Icon(Icons.calendar_view_month)),
              ButtonSegment<Calendar>(
                  value: Calendar.year,
                  label: Text('Year'),
                  icon: Icon(Icons.calendar_today)),
            ],
            selected: <Calendar>{calendarView},
            onSelectionChanged: (Set<Calendar> newSelection) {
              setState(() {
                calendarView = newSelection.first;
              });
            },
          ),
        ),
      ),
    );
  }
}

```

</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

3 participants