Skip to content

Implement AlertDialog title/content overflow scroll#43226

Merged
shihaohong merged 6 commits intoflutter:masterfrom
shihaohong:alert-dialog-scroll
Oct 28, 2019
Merged

Implement AlertDialog title/content overflow scroll#43226
shihaohong merged 6 commits intoflutter:masterfrom
shihaohong:alert-dialog-scroll

Conversation

@shihaohong
Copy link
Contributor

@shihaohong shihaohong commented Oct 22, 2019

Description

When the title and content are too tall in an AlertDialog, the title can end up overflowing while the content will be clipped. An example of this occurring is when the text scale factor is increased, causing parts of the alert dialog to be obscured.

This PR introduces a scroll view such that the title and content of an AlertDialog is scrollable so that its entire content can be viewed if need be.

Normal size behavior:

Screen Shot 2019-10-21 at 4 56 01 PM

Original implementation causing overflow:

Screen Shot 2019-10-21 at 4 57 49 PM

Overflow Fix:

Screen Shot 2019-10-21 at 4 59 22 PM

scroll_alert_dialog

Related Issues

Fixes part of #42696

Tests

I added the following tests:

  • Tests to ensure that both the title and content can be dragged and scrolled in an AlertDialog

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). Replace this with a link to the e-mail where you asked for input on this proposed change.
  • No, this is not a breaking change.

@shihaohong shihaohong added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) labels Oct 22, 2019
@shihaohong shihaohong requested a review from HansMuller October 22, 2019 00:01
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

Nice new feature!

style: contentTextStyle ?? dialogTheme.contentTextStyle ?? theme.textTheme.subhead,
child: content,
child: SingleChildScrollView(
child: Column(
Copy link
Contributor

Choose a reason for hiding this comment

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

I was surprised that MainAxisSize.min isn't needed here. It looks like it's not needed because none of the children are Expanded.

Copy link
Contributor Author

@shihaohong shihaohong Oct 22, 2019

Choose a reason for hiding this comment

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

It turns out that you were right -- not setting the mainAxisSize and crossAxisAlignment settings for the Column was breaking a DialogTheme golden test. I just implemented the fix.

@shihaohong shihaohong added c: API break Backwards-incompatible API changes and removed c: API break Backwards-incompatible API changes labels Oct 22, 2019
@shihaohong shihaohong merged commit c461ff9 into flutter:master Oct 28, 2019
@shihaohong shihaohong deleted the alert-dialog-scroll branch October 28, 2019 20:45
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019
* Wrap alert dialog title and content in single child scroll view

* Scrollable alert dialog title and content tests

* Remove unnecessary comment

* Fix mainAxisSize and crossAxisAlignment issue
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019
@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

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) 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