Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[AutoComplete] Add callback signatures to property docs #3550

Merged
merged 1 commit into from
Mar 4, 2016
Merged

[AutoComplete] Add callback signatures to property docs #3550

merged 1 commit into from
Mar 4, 2016

Conversation

theosherry
Copy link
Contributor

Hey, this is more work towards closing #3096

I had to add an ignore comments flag to the max-len lint rule; if you break up a @param tag into multiple lines it causes errors in the parsing. I'm not sure if this is the best way to fix the problem, so let me know what you think.

oliviertassinari
oliviertassinari reviewed Mar 1, 2016
View reviewed changes
.eslintrc
@@ -30,7 +30,7 @@ rules:
jsx-quotes: [2, prefer-double]
keyword-spacing: 2
key-spacing: 2
max-len: [2, 120, 4]
max-len: [2, 120, 4, {"ignoreComments": true}]
Copy link
Member

Choose a reason for hiding this comment

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

I'm again this change.
It's the minimum value so that any line fit into the GitHub web view.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you suggest instead?

@theosherry
Copy link
Contributor Author

@oliviertassinari I've removed the lint rule change and now the new lines in @param or @returns descriptions are removed in PropTypeDescription. Let me know what you think.

@oliviertassinari
Copy link
Member

Breaking the description in multiple lines seems better. Thanks 👍.
The complexity of PropTypeDescription has increased.
At some point, we will need to add some unit tests.

@theosherry
Copy link
Contributor Author

Thanks @oliviertassinari , I agree about the unit tests onPropTypeDescription. I tried to write one for the original changes I made in adding @returns parsing, but I couldn't figure out how to get some raw test code passed into the component. I can look at it again when I have some time.

@mbrookes mbrookes mentioned this pull request Mar 2, 2016
Merged
@theosherry theosherry changed the title [Docs] Added callback signatures for AutoComplete documentation [AutoComplete] Add callback signatures to property docs Mar 2, 2016
@mbrookes
Copy link
Member

mbrookes commented Mar 2, 2016

@theosherry - sorry to be a nuisance, but please could you split the PropTypeDescription update into it's own PR?

@mbrookes mbrookes added the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Mar 2, 2016
@mbrookes mbrookes self-assigned this Mar 2, 2016
@theosherry
Copy link
Contributor Author

Sure, no problem

@theosherry theosherry mentioned this pull request Mar 2, 2016
Merged
@mbrookes mbrookes added on hold There is a blocker, we need to wait PR: Review Accepted and removed PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI on hold There is a blocker, we need to wait labels Mar 2, 2016
oliviertassinari added a commit that referenced this pull request Mar 4, 2016
@oliviertassinari
Merge pull request #3550 from theosherry/autocomplete_docs
995b66a 
[AutoComplete] Add callback signatures to property docs
@oliviertassinari oliviertassinari merged commit 995b66a into mui:master Mar 4, 2016
@oliviertassinari
Copy link
Member

@theosherry Thanks!

mbradleyis pushed a commit to staticinstance/material-ui that referenced this pull request Mar 7, 2016
@oliviertassinari
Merge pull request mui#3550 from theosherry/autocomplete_docs
9916a2e 
[AutoComplete] Add callback signatures to property docs
@mbrookes mbrookes mentioned this pull request Mar 24, 2016
Closed
37 tasks
@zannager zannager added docs Improvements or additions to the documentation component: autocomplete This is the name of the generic UI component, not the React module! labels Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@mbrookes mbrookes

Labels
component: autocomplete This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@theosherry @oliviertassinari @mbrookes @zannager