Skip to content

Make expansion panel optionally toggle its state by tapping its header. #29390

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.

Sign up for GitHub

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

Jump to bottom
Merged
shihaohong merged 1 commit into from
Apr 18, 2019
Merged

Make expansion panel optionally toggle its state by tapping its header. #29390

shihaohong merged 1 commit into from
Apr 18, 2019

Conversation

@esarbanis
Copy link
Contributor

@esarbanis esarbanis commented Mar 14, 2019

Description

ExpansionPanels can toggle their expand/collapse state only by tapping on the ExpandIcon. This change introduces an optional parameter to the ExpansionPanel that can make it toggle its state by tapping anywhere in its header widget.

Related Issues

Fixes #5845

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.
  • My PR includes tests for all changed/updated/fixed behaviors (See Test Coverage).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • 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?

@esarbanis esarbanis force-pushed the tappable-expansion-panel-header branch 2 times, most recently from 0aba484 to 1d10abe Compare March 15, 2019 17:30
@goderbauer goderbauer added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Mar 15, 2019
@esarbanis esarbanis force-pushed the tappable-expansion-panel-header branch from 1d10abe to cf3a205 Compare March 16, 2019 16:18
@HansMuller HansMuller added the c: new feature Nothing broken; request for a new capability label Apr 4, 2019
@shihaohong shihaohong self-requested a review April 5, 2019 15:28
shihaohong
shihaohong previously requested changes Apr 5, 2019
View reviewed changes
Copy link
Contributor

@shihaohong shihaohong left a comment

Choose a reason for hiding this comment

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

Overall, this is a good setup. I'd pay special attention to the ink splash effects and making sure you have a defined behavior in your mind before jumping in the code, since that part can be trickier.

@shihaohong shihaohong requested a review from HansMuller April 5, 2019 16:40
@shihaohong
Copy link
Contributor

shihaohong commented Apr 5, 2019

cc/ @HansMuller

@esarbanis esarbanis force-pushed the tappable-expansion-panel-header branch from cf3a205 to 4597d17 Compare April 7, 2019 11:10
shihaohong
shihaohong reviewed Apr 10, 2019
View reviewed changes
@shihaohong shihaohong self-requested a review April 10, 2019 21:24
@shihaohong shihaohong dismissed their stale review April 10, 2019 21:26

Waiting on response for discussion, neither explicitly approving or requesting change

@esarbanis esarbanis force-pushed the tappable-expansion-panel-header branch 3 times, most recently from 2b13aa4 to 833fb5d Compare April 12, 2019 07:30
shihaohong
shihaohong reviewed Apr 13, 2019
View reviewed changes
Copy link
Contributor

@shihaohong shihaohong left a comment

Choose a reason for hiding this comment

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

I have a few more suggestions regarding syntax and adding a few more tests to be more exhaustive.

@shihaohong
Copy link
Contributor

shihaohong commented Apr 13, 2019

Also, don't forget to run the tests to see if they work and analyze the code, it seems you have some failed pre-submit tests

@esarbanis esarbanis force-pushed the tappable-expansion-panel-header branch 3 times, most recently from 2eac013 to 71bd8bd Compare April 17, 2019 09:26
shihaohong
shihaohong approved these changes Apr 17, 2019
View reviewed changes
Copy link
Contributor

@shihaohong shihaohong left a comment

Choose a reason for hiding this comment

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

Other than the test description changes, LGTM

@esarbanis esarbanis force-pushed the tappable-expansion-panel-header branch from 71bd8bd to d69197f Compare April 18, 2019 07:59
@shihaohong shihaohong merged commit 2e6edaf into flutter:master Apr 18, 2019
@shihaohong
Copy link
Contributor

shihaohong commented Apr 18, 2019

Thank you for your work with this @esarbanis!

@esarbanis
Copy link
Contributor Author

esarbanis commented Apr 19, 2019

@shihaohong thank you for the time spent in reviewing!

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

Reviewers

@HansMuller HansMuller Awaiting requested review from HansMuller

1 more reviewer

@shihaohong shihaohong shihaohong approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Consider making expansion panels clickable.

5 participants

@esarbanis @shihaohong @goderbauer @HansMuller @googlebot