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

✨ Add 'getPreviewIcon' to DropzoneArea #154

Merged
merged 2 commits into from
May 5, 2020

Conversation

max-carroll
Copy link
Contributor

@max-carroll max-carroll commented May 4, 2020

Description

I have created a new prop getPreviewIcon which allows you to pass your own function which determines which Preview Icon should be displayed for selected file.

I also added an example in the markdown as this was useful for testing, and thought it would also be useful so that people can see how to use the prop and a possible use case for it

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested

  • Test A => With the default getPreviewIcon prop I verified that an image file dragged on would display the image preview, as per before
  • Test B => With the default getPreviewIcon prop I verified that a file type which wasn't an image dragged on would display a paper clip icon, as per before
  • Test C => I wrote and extra example in the markdown with a custom function which demonstrates we can pass in a function for determining the icon. I distinguished between Audio, Video, MS Word Documents and Pdfs in order to determine specific icons

Test Configuration:

  • Browser:

Windows 10, Chrome Browser

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

image

There is a linting error, not sure how to get around this, any ideas?

  • I have added tests that prove my fix is effective or that my feature works

There appears to be no testing framework in place

  • New and existing unit tests pass locally with my changes

n/a see above

  • Any dependent changes have been merged and published in downstream modules

n/a

@max-carroll max-carroll changed the title Max work 3 ✨ Add 'getPreviewIcon' to DropzoneArea May 4, 2020
Copy link
Contributor

@panz3r panz3r left a comment

Choose a reason for hiding this comment

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

Hi @max-carroll ,

Thanks for the PR, looks good but I marked some possible changes.

src/components/DropzoneArea.js Outdated Show resolved Hide resolved
src/components/DropzoneArea.js Show resolved Hide resolved
src/components/DropzoneArea.js Outdated Show resolved Hide resolved
src/components/DropzoneArea.js Outdated Show resolved Hide resolved
src/components/PreviewList.js Outdated Show resolved Hide resolved
src/components/DropzoneArea.md Show resolved Hide resolved
@panz3r panz3r added this to the 3.1 milestone May 5, 2020
@max-carroll
Copy link
Contributor Author

Hi there @panz3r I want to fix this now, I will work on fork, should this be as a new commit, or should I modify the old commit, can you help me with the workflow a bit please?

@panz3r
Copy link
Contributor

panz3r commented May 5, 2020

Hi there @panz3r I want to fix this now, I will work on fork, should this be as a new commit, or should I modify the old commit, can you help me with the workflow a bit please?

Hi @max-carroll ,

I'd try to keep it in the old commit, also (since you'll have to rebase a bit 😬 ) reword the second commit to Add example of how to use the 'getPreviewIcon' prop.
The "simplest" way I see is to do the following:

  • create a new commit with the fixes
  • start an interactive rebase
  • change the commits order and squash the "fix" commit with the "feature" one
  • reword the "docs" commit
  • complete the rebase

(You'll need to force push afterward but that's ok)

@max-carroll
Copy link
Contributor Author

Thanks for your guidance on git rebase interactive, I have now learnt how to do this and done it. My work has been force pushed to the branch max-work-3 where I created this pull request from, whats the next step? Thanks for being patient with me I'm just learning the workflow 😄

Copy link
Contributor

@panz3r panz3r left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@panz3r
Copy link
Contributor

panz3r commented May 5, 2020

whats the next step?

Going to merge this into master soon, in the meanwhile you can backport the changes onto v2.x and open a new PR for that branch.

Usually the workflow for the backport is as easy as:

  • checkout branch v2.x
  • create a new branch starting for v2.x
  • cherrypick changes from the other feature branch
  • open a new PR pointing to v2.x

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