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.

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

Updated RefreshControl component #61

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ani4aniket
Copy link
Member

Fixes #57

Summary

  • Updated the title and prop description from docs.
  • Different approach to show inheritedProps of the component.
    • It is redundant to use @inherits annotations to show inherited props as inheritedProps is already getting parsed as composes(Array of string) by react-docgen. We can collectively store links of all the inheritedProps into a file and then outsource their exposed URL depending on the value of composes
      Screenshot 2020-06-22 at 4 08 28 AM
  • Figure out the way to insert links within the props table, in this component colorprop which is repeated multiple times in the md file.

@ani4aniket ani4aniket added the documentation Improvements or additions to documentation label Jun 22, 2020
@jevakallio
Copy link

Different approach to show inheritedProps of the component.

Ooh, conceptually I like this! The downside is that the code needs to be structured in a way that react-docgen understand. In flowtype there's a couple of different ways of creating combination types. ...spreading them like in this example is one, doing a type union like this is another:

  type Props = ViewProps | ButtonProps | AndroidProps | IOSProps.

@ani4aniket could you check if any other components use this method, or some alternative, and if react-docgen works with it. I really like it, we just don't want someone to break docs accidentally by doing a refactoring!

(I'll do a code review later, now on a call)

@ani4aniket
Copy link
Member Author

ani4aniket commented Jun 22, 2020

Yes, I have verified and tested this in many components, in every component the inherited props are parsed as composes using react-docgen. Also, I have noticed that iOS and Android props are ignored while parsing as it can be seen in the above screenshot.
You can also verify that too https://reactcommunity.org/react-docgen/.

Copy link

@jevakallio jevakallio left a comment

Choose a reason for hiding this comment

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

LGTM! :shipit: Let's open a PR to upstream!

@ani4aniket ani4aniket force-pushed the components/RefreshControl branch from 416964c to 552cbce Compare August 5, 2020 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update the RefreshControl component
2 participants