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

fix(#2662): remove many instances of default props #2752

Merged
merged 3 commits into from
Jun 5, 2023

Conversation

amillward
Copy link
Contributor

  • Bug fix
  • New feature
  • Chore
  • Breaking change
  • There is an open issue which this change addresses
  • I have read the CONTRIBUTING document.
  • My commits follow the Git Commit Guidelines
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • My change requires a change to Typescript typings.
    • I have updated the typings accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Partially fixes #2662

@amillward
Copy link
Contributor Author

I'm aware this doesn't fully address defaultProps for all components (I am too time-poor right now to do the more complex cases) but hopefully this is a start as this warning is now extremely noisy

@amillward amillward marked this pull request as ready for review May 15, 2023 19:35
@amillward amillward changed the title fix(2662): remove many instances of default props fix(#2662): remove many instances of default props May 15, 2023
src/Fade.js Outdated Show resolved Hide resolved
@singleseeker
Copy link

Hope it can be merged ASAP.

Copy link

@singleseeker singleseeker left a comment

Choose a reason for hiding this comment

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

Cool for removing the defaultProps.

src/NavbarToggler.js Outdated Show resolved Hide resolved
src/Toast.js Outdated Show resolved Hide resolved
src/PlaceholderButton.js Outdated Show resolved Hide resolved
@illiteratewriter
Copy link
Member

Thank you so much for the PR.

I added comments earlier instead of a review. Sorry about that.

Also, Reactstrap uses eslint for linting, and prettier for code styles.
If you run yarn lint before making any code changes now, you'll be able to see a couple of errors. Eslint helps with such small errors.
Also, once you're done; run yarn prettier. It keeps code style consistent across the project.

@amillward
Copy link
Contributor Author

@illiteratewriter thanks for the feedback. I have responded to them all. Happy to discuss any further :)

@amillward amillward force-pushed the remove-default-props branch 2 times, most recently from 11aeecc to 53aadfd Compare May 27, 2023 13:56
@singleseeker
Copy link

@illiteratewriter Hi, what's next? Do you plan to merge this PR?

@illiteratewriter illiteratewriter merged commit b7d571c into reactstrap:master Jun 5, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants