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

Added Image Loading Customisation #155

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

Conversation

LeoMacherla
Copy link

Features

  • Added option to hide default loading (react activity indicator)
  • Added option to add custom loading component
  • Added option to pass onImageLoad which is called when the image loads

Notes

This is my first time contributing to an open source project so I hope I've done everything correctly :)

@jforaker
Copy link

jforaker commented Aug 9, 2022

so I hope I've done everything correctly

Hey so this is cool, however there's one thing that is not quite correct..

By the looks of the diff in github, it seems you changed the entire project because your IDE added tabs and some interesting spacing. Also you changed the quotes to single. Therefore it looks like you added 564 new lines, but you certainly did not.

You should try to match the project editor style so next time your PR would only show the changes you made. Granted this project does not have shared configuration (like Prettier), so it is a little bit harder. However you can adjust your text editor/IDE preferences and try and match the source code a little better. Because right now, it is impossible to see what your intentional changes are vs what your IDE changed.

@LeoMacherla
Copy link
Author

so I hope I've done everything correctly

Hey so this is cool, however there's one thing that is not quite correct..

By the looks of the diff in github, it seems you changed the entire project because your IDE added tabs and some interesting spacing. Also you changed the quotes to single. Therefore it looks like you added 564 new lines, but you certainly did not.

You should try to match the project editor style so next time your PR would only show the changes you made. Granted this project does not have shared configuration (like Prettier), so it is a little bit harder. However you can adjust your text editor/IDE preferences and try and match the source code a little better. Because right now, it is impossible to see what your intentional changes are vs what your IDE changed.

Oh okay I see. Sorry! I'll fix that and create a new pull request with just the changes I intended. Thanks for pointing that out 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants