Skip to content

Conversation

@vpaladino778
Copy link
Contributor

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 shows hintText. This PR corrects that change so that the code snippet matches the image.

image

@flutter-dashboard flutter-dashboard bot added a: text input Entering text in a text field or keyboard related problems f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Nov 23, 2021
@flutter-dashboard
Copy link

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.

@google-cla google-cla bot added the cla: yes label Nov 23, 2021
@vpaladino778 vpaladino778 changed the title Modified labelText to hintText Modified TextField docs - Replaced 'labelText' to 'hintText' in code snippet Nov 23, 2021
@Hixie
Copy link
Contributor

Hixie commented Nov 23, 2021

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.

Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

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.

@vpaladino778
Copy link
Contributor Author

@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.

@gspencergoog
Copy link
Contributor

@vpaladino778
Copy link
Contributor Author

I fixed the naming convention for the sample apps. Renamed text_field.1.dart to text_field.0.dart, and named the new sample app text_field.1.dart.

Copy link
Contributor

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.

Copy link
Contributor Author

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

Copy link
Contributor

@gspencergoog gspencergoog left a 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

@flutter-dashboard
Copy link

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 package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@Hixie
Copy link
Contributor

Hixie commented Feb 16, 2022

@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.

@vpaladino778
Copy link
Contributor Author

@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.

@Hixie
Copy link
Contributor

Hixie commented Apr 26, 2022

@vpaladino778 Just checking in, anything we can do to help?

@guidezpl guidezpl removed the cla: yes label May 1, 2022
@Hixie
Copy link
Contributor

Hixie commented Jul 12, 2022

cc @goderbauer

@vpaladino778
Copy link
Contributor Author

@vpaladino778 Just checking in, anything we can do to help?

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

@Hixie
Copy link
Contributor

Hixie commented Sep 27, 2022

@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!

@Hixie
Copy link
Contributor

Hixie commented Dec 14, 2022

@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.

@Hixie
Copy link
Contributor

Hixie commented Mar 28, 2023

@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.

@goderbauer goderbauer force-pushed the vpaladino778-fix-textfield-docs branch from 6f50645 to 849d029 Compare March 28, 2023 23:45
@goderbauer goderbauer requested a review from gspencergoog March 28, 2023 23:46
@vpaladino778
Copy link
Contributor Author

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

@goderbauer
Copy link
Member

@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!

@goderbauer
Copy link
Member

This is now blocked on #123669.

@goderbauer goderbauer added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 29, 2023
@auto-submit auto-submit bot merged commit f440649 into flutter:master Mar 29, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 30, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 30, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 30, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 30, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 30, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 30, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2023
exaby73 pushed a commit to NevercodeHQ/flutter that referenced this pull request Apr 17, 2023
…snippet (flutter#94128)

Modified TextField docs - Replaced 'labelText' to 'hintText' in code snippet
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App 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.

5 participants