-
Notifications
You must be signed in to change notification settings - Fork 29.7k
[EditableText] honor the "brieflyShowPassword" system setting #97769
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
[EditableText] honor the "brieflyShowPassword" system setting #97769
Conversation
justinmc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick question in the tests where a value was not what I expected. Probably I just didn't follow the test correctly. Otherwise ready to LGTM.
I'm glad we will have this feature now!
| await tester.enterText(find.byType(EditableText), 'AAA'); | ||
| await tester.pump(); | ||
|
|
||
| tester.binding.window.brieflyShowPasswordTestValue = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's cool that we can easily fake this stuff in tests, I haven't worked much with WidgetsBinding before.
| addTearDown(() { | ||
| tester.binding.window.brieflyShowPasswordTestValue = true; | ||
| }); | ||
| expect((findRenderEditable(tester).text! as TextSpan).text, '••A'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait why is the final "A" visible if brieflyShowPasswordTestValue is false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it becomes hidden on the next cursor tick.
| bool get brieflyShowPassword => _brieflyShowPasswordTestValue ?? platformDispatcher.brieflyShowPassword; | ||
| bool? _brieflyShowPasswordTestValue; | ||
| /// Hides the real [brieflyShowPassword] and reports the given | ||
| /// `brieflyShowPassword` instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Maybe say brieflyShowPasswordTestValue instead of brieflyShowPassword?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right good catch!
justinmc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
|
This was reverted in #98089 |
…flutter#97769)" (flutter#98089) This reverts commit 63f48d1.
Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.
List which issues are fixed by this PR. You must list at least one issue.
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.