-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Refactor: updated segmented button #129215
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
Conversation
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.
|
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. |
|
HI @AcarFurkan 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. |
|
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. |
|
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 Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
@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! |
|
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. |
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>
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.
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
///).