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
[NO JIRA]: Updating propTypes package and fixing incorrect proptypes #2261
Conversation
Browser supportIf this is a visual change, make sure you've tested it in multiple browsers, |
Visual regression tests passed 😎. Bear in mind that they only run in Chromium on static components – they aren't perfect. |
Visit https://backpack.github.io/storybook-prs/2261 to see this build running in a browser. |
@@ -75,8 +75,8 @@ const propTypes = { | |||
loaderIntersectionTrigger: PropTypes.oneOf(['small', 'half', 'full']), | |||
onScroll: PropTypes.func, | |||
onScrollFinished: PropTypes.func, | |||
renderLoadingComponent: PropTypes.func, |
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.
Both of these are actually treated as functions, not React components so this is not correct. That said I don't think there's a down side to start treating them as components
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 yes I see, maybe it does make sense to keep it as that given how its used in comparios to other things
Co-authored-by: Hugo Tunius <h@tunius.se>
@@ -170,8 +170,8 @@ Updates the internal array and triggers all listeners. | |||
| loaderMinDisplay | oneOf(['small', 'half', 'full']) | false | 'full' | | |||
| onScroll | func | false | null | | |||
| onScrollFinished | func | false | null | | |||
| renderLoadingComponent | func | false | null | |
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.
This should be changed back to func
too
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 catch! Solved!
This PR solves two things:
propTypes
to latest version to allow use ofelementType
prop (see more)func
but in newer React versions its not strictly the caseRemember to include the following changes:
UNRELEASED.md
(See How to write a good changelog entry)README.md