Skip to content

Conversation

@shihaohong
Copy link
Contributor

@shihaohong shihaohong commented Sep 5, 2019

Description

Currently, there is no way to control the constraints of each button in the ToggleButtons widget. It currently defaults to const BoxConstraints(minWidth: 88.0, minHeight: 36.0), which is the constraints for RawMaterialButton, but this is undesirable if we want it to look like the square toggle buttons.

image

This PR does the following:

  1. Sets ToggleButtons' default BoxConstraints to BoxConstraints(minWidth: 48.0, minHeight: 48.0)
  2. Adds a constraints parameter to ToggleButtons and ToggleButtonsTheme

Required Follow-up

  • Update flutter/assets-for-api-docs to ensure that the defaults in the code sample are updated

Related Issues

Fixes #39216

Tests

I added the following tests:

  1. Test for ToggleButtons default and custom constraints
  2. Test for custom ToggleButtonsTheme
  3. Updated existing test to account for new constraints parameter

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (Please read Handling breaking changes). - TODO: send email and link here once LGTM is received
  • No, this is not a breaking change.

@shihaohong shihaohong added c: new feature Nothing broken; request for a new capability framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Sep 5, 2019
Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

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

LGTM

}

final TextStyle currentTextStyle = textStyle ?? toggleButtonsTheme.textStyle ?? theme.textTheme.body1;
final BoxConstraints currentConstraints = constraints ?? toggleButtonsTheme.constraints ?? const BoxConstraints(minWidth: 48.0, minHeight: 48.0);
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use kMinInteractiveDimension here

/// Typically used to constrain the button's minimum size.
///
/// If this property is null, then
/// BoxConstraints(minWidth: 48.0, minHeight: 48.0) is be used.
Copy link
Contributor

Choose a reason for hiding this comment

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

Although the 48 is kMinInteractiveDimension I think the documentation is easier to understand the way you have it.

@shihaohong shihaohong merged commit 2bd7f9f into flutter:master Sep 13, 2019
@shihaohong shihaohong deleted the toggle-buttons-constraints branch September 13, 2019 15:19
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Sep 30, 2019
…meter (flutter#39857)

* Add constraints property, updated default constraints for ToggleButtons to 48x48

* Add kMinInteractiveDimension constant to ToggleButtons
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

c: new feature Nothing broken; request for a new capability 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.

Add ToggleButtons.constraints parameter

3 participants