-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Modified TextField docs - Replaced 'labelText' to 'hintText' in code snippet #94128
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
Modified TextField docs - Replaced 'labelText' to 'hintText' in code snippet #94128
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
test-exempt: only affects comments That said, this is actually testable; if the sample snippet is turned into an actually piece of sample code (like most other samples) then it'd be run and analyzed and we would catch this kind of problem. |
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.
I'm happy to accept this update, but as @Hixie pointed out, it would be great to convert this to a "dartpad" sample. That is a bit more work to do, though, so I understand if you're not interested.
|
@gspencergoog Hey Greg, I'd be happy to give it a go. I'll rerequest a review from you once I got it figured out. |
|
OK, great! Take a look at https://github.com/flutter/flutter/blob/master/examples/api/README.md for info on how to do it. |
|
I fixed the naming convention for the sample apps. Renamed |
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.
Why isn't this one text_field.0.dart? It comes first in the comments, so it should have the lower index.
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.
Good point, I agree. I'll make the switch
gspencergoog
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.
OK, this is great, now the only thing missing is a test for the new example.
You can see an example of what a test might look like here:
https://github.com/flutter/flutter/blob/master/examples/api/test/widgets/layout_builder/layout_builder.0_test.dart
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
@vpaladino778 Sorry to make you jump through all these hoops for a typo fix... is there any chance you are able to add a test like @gspencergoog suggested? No worries if not, we can take it from here if necessary. |
Yeah i'm up for it. I had gotten started a while back but mostly forgot to finish it up. I can take a look this weekend and hopefully have the test working. |
|
@vpaladino778 Just checking in, anything we can do to help? |
|
cc @goderbauer |
Hey there, I can take a look at this tonight. Sorry for the delay, just started a new job and have been busy with that. Last time i tried getting the test working I had some issues with my local development environment. My Android studio only shows the 'java' folder and no files. And the dart file i have open has 'Target URI not found' errors all over it, also getting 'Pub get has not been run'. So i think something wacky is going on with my IDE. I'm going to try and update android studio and reimport the project. Let me know if you have any suggestions |
|
@vpaladino778 congratulations on the new job! If you do have time to work on this please feel free to reach out on our Discord to get help with writing the test, the #hackers-new channel is the best place to reach out. Thanks! |
|
@vpaladino778 I feel bad that we made you jump through all those hoops and as a result the docs are still bad even though you sent us a fix. Can you let me know if you still have time to look at this? It's fine if you don't, we can take it over and write the test for it. |
|
@goderbauer any chance you can land this for @vpaladino778? i feel really bad about making him jump through all these hoops just for a simple typo fix. |
6f50645 to
849d029
Compare
|
Hey @Hixie ! Sorry for the delay on this, i tried to write the test but my whole local dev environment was messed up so i wasnt able to get it working. Ya'll are more than welcome to push up the required change to my branch |
|
@vpaladino778 No worries, I just pushed a commit to this branch that resolves the outstanding comments. Once @gspencergoog has reviewed it, this should be ready to land. Thanks for your contribution! |
|
This is now blocked on #123669. |
…snippet (flutter#94128) Modified TextField docs - Replaced 'labelText' to 'hintText' in code snippet

Below is a screenshot of the current state of the TextField docs. As you can see, the code snippet shows
labelText, but the image that is attached showshintText. This PR corrects that change so that the code snippet matches the image.