-
Notifications
You must be signed in to change notification settings - Fork 161
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
Update types and documentation regarding isIntrinsicHeight #358
Update types and documentation regarding isIntrinsicHeight #358
Conversation
@@ -15,8 +15,10 @@ const Slide = class Slide extends React.PureComponent { | |||
index: PropTypes.number.isRequired, | |||
innerClassName: PropTypes.string, | |||
innerTag: PropTypes.string, | |||
naturalSlideHeight: PropTypes.number.isRequired, | |||
naturalSlideWidth: PropTypes.number.isRequired, | |||
/* eslint-disable react/require-default-props */ |
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.
Just a question, is there a way to accomplish this without ignoring the rule?
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.
Not sure to be honest. I am pretty sure I tried, but basically CarouselPropTypes.slideSize
can be either optional or required and eslint struggles with that.
But the main takeaway is that:
- If it's required, then it doesn't need default props
- If it's optional, then default doesn't need to be anything as it can be undefined (it doesn't matter)
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This issue has been automatically closed becuase it has not had recent activity. |
Could we reopen? :) Don't want my work to go for nothing please |
@ppozniak Thanks for the call out. I am re-opening this PR and it is on my to-do list. Sorry for the delays. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This issue has been automatically closed becuase it has not had recent activity. |
should this be re opened? @captnstarburst |
What:
#356
Update types to make
naturalSlideWidth
and height optional whenisIntrensicHeight
is trueWhy:
To avoid confusion and do not force user to set props that are not needed in certain situations.
How:
Changes to documentation, TS typings and
PropTypes
Additionally I've deleted unused custom prop type
height
Checklist: