Skip to content

Reland "Nnbd widgets"#65528

Merged
a14n merged 4 commits intoflutter:masterfrom
a14n:nnbd-widgets
Sep 15, 2020
Merged

Reland "Nnbd widgets"#65528
a14n merged 4 commits intoflutter:masterfrom
a14n:nnbd-widgets

Conversation

@a14n
Copy link
Contributor

@a14n a14n commented Sep 10, 2020

Description

Related Issues

@flutter-dashboard flutter-dashboard bot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels. labels Sep 10, 2020
@a14n a14n requested a review from goderbauer September 10, 2020 07:17
goderbauer
goderbauer approved these changes Sep 11, 2020
View reviewed changes
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@goderbauer
Copy link
Member

goderbauer commented Sep 11, 2020 •
edited
Loading

Note for google3: cl/330814656 must be applied when this rolls into google3 (fix is scheduled in FROB).

@goderbauer
Copy link
Member

goderbauer commented Sep 11, 2020

This change also requires dart-lang/sdk@8008c8d, which has already landed in Flutter.

@a14n
Copy link
Contributor Author

a14n commented Sep 14, 2020

Google testing — Google testing failed. Please contact a Google developer to investigate.

@goderbauer feel free to merge this PR when you're available to follow up on google3

mit-mit
mit-mit reviewed Sep 14, 2020
View reviewed changes
packages/flutter/lib/src/widgets/text.dart Outdated
Copy link
Member

@mit-mit mit-mit Sep 14, 2020

Choose a reason for hiding this comment

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

@a14n, could this be String this.data so that you can't pass nulls to the default constructor (i.e. Text(null) should be an error) ?

Copy link
Contributor Author

@a14n a14n Sep 14, 2020

Choose a reason for hiding this comment

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

Sure! Thanks for the review :) A lint could make sense to catch similar issue.

Copy link
Contributor Author

@a14n a14n Sep 14, 2020

Choose a reason for hiding this comment

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

Done. I also changed Text.rich textSpan parameter.

Copy link
Contributor Author

@a14n a14n Sep 14, 2020

Choose a reason for hiding this comment

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

Filed dart-lang/linter#2237

@goderbauer
Copy link
Member

goderbauer commented Sep 14, 2020

"google testing" failure was probably caused by an internal merge conflict. Should be resolved. I restarted the tests. Let's see what happens.

@goderbauer
Copy link
Member

goderbauer commented Sep 14, 2020

"google testing" is passing. I've marked it as waiting for the tree to go green.

@goderbauer goderbauer mentioned this pull request Sep 14, 2020
Closed
@a14n a14n merged commit b007a81 into flutter:master Sep 15, 2020
@a14n a14n deleted the nnbd-widgets branch September 15, 2020 14:42
@goderbauer goderbauer mentioned this pull request Sep 15, 2020
Closed
13 tasks
@goderbauer
Copy link
Member

goderbauer commented Sep 15, 2020

This should fix the test failure: #65860.

@a14n a14n mentioned this pull request Sep 15, 2020
Merged
@Zazo032 Zazo032 mentioned this pull request Sep 21, 2020
Closed
@dnfield dnfield mentioned this pull request Apr 6, 2021
Open
@Piinks Piinks mentioned this pull request Aug 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mit-mit mit-mit mit-mit left review comments

@goderbauer goderbauer goderbauer approved these changes

Assignees

No one assigned

Labels

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

4 participants

@a14n @goderbauer @mit-mit @googlebot