Skip to content
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

Use OverflowBar instead of ButtonBar in TimePicker #62601

Merged

Conversation

HansMuller
Copy link
Contributor

@HansMuller HansMuller commented Jul 30, 2020

Removed TimePicker's dependency on ButtonBar, which has its own troublesome dependencies on ButtonTheme (now obsolete - see flutter.dev/go/material-button-system-updates) as well as a problematic implementation, see #53378.

This change is similar to #62686

@fluttergithubbot fluttergithubbot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Jul 30, 2020
await tester.tap(find.text('OK'));
await tester.pumpAndSettle();
});

// TODO(rami-a): Re-enable and fix test.
Copy link
Member

Choose a reason for hiding this comment

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

Oops, looks like I forgot to remove a TODO, do you think you could remove it as part of this PR? Looks like the formatting of that test is a bit off too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, done.

child: Text(widget.confirmText ?? localizations.okButtonLabel),
),
],
child: Container(
Copy link
Member

Choose a reason for hiding this comment

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

Will this result in any visual diff from the ButtonBar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so. I've already done some image diffs before and after this PR, to check. Still need to check the test repo ofcourse.

@HansMuller HansMuller force-pushed the replace_button_bar_in_time_picker branch from f209173 to 2a350e1 Compare August 1, 2020 00:17
@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@HansMuller HansMuller merged commit 17d3179 into flutter:master Aug 3, 2020
@HansMuller HansMuller deleted the replace_button_bar_in_time_picker branch August 3, 2020 21:26
Pragya007 pushed a commit to Pragya007/flutter that referenced this pull request Aug 11, 2020
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.

None yet

4 participants