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

feat: add onClick event to PDFDownloadLink #1902

Merged
merged 3 commits into from
Jul 2, 2022

Conversation

adamduncan
Copy link
Contributor

Based on request #975 and our own experience, having an onClick handler to invoke for PDFDownloadLink would be benficial. E.g. We'd like to send a custom event to our analytics on download. (We're finding other means to lazily load the heavier components, so wouldn't need to resort to the workaround for this feature proposed there.)

This implementation refactors the existing onClick handler slightly, to also allow consumer's to provide a function via props. We conditionally call this function after existing handleDownloadIE. (We'd generally opt for onClick?.(), but as this package targets Node v12, we'll check typeof before invoking.)

I've included the event as the first function argument, should the consumer need to use native event properties. That seems like the expected behaviour. Secondly, we pass along the instance, which might be helpful in certain circumstances. If that's not a beneficial use-case, would gladly omit.

Added types. Not sure that this component has unit tests, but happy to add if need be.

Fixes #975

@diegomura diegomura changed the title feat: Add onClick event to PDFDownloadLink feat: add onClick event to PDFDownloadLink Jul 2, 2022
@diegomura diegomura merged commit d011983 into diegomura:master Jul 2, 2022
@diegomura
Copy link
Owner

Thanks so much!

@github-actions github-actions bot mentioned this pull request Jul 2, 2022
@aclec
Copy link

aclec commented Jul 6, 2022

Hi, I have an issue with your Type in TS:

node_modules/@react-pdf/renderer/index.d.ts:451:15 - error TS2552: Cannot find name 'function'. Did you mean 'Function'?

451     onClick?: function;
                  ~~~~~~~~

  node_modules/typescript/lib/lib.es5.d.ts:324:13
    324 declare var Function: FunctionConstructor;
                    ~~~~~~~~
    'Function' is declared here.


Found 1 error in node_modules/@react-pdf/renderer/index.d.ts:451

@adamduncan
Copy link
Contributor Author

@aclec My mistake. Rushed that one through. PR here #1924

@miladehghani
Copy link

@adamduncan I still can see the issue @aclec mentioned on @react-pdf/renderer@2.3.0 have you fixed it on that version?

@adamduncan
Copy link
Contributor Author

@miladehghani I believe the PR has been merged into master but a fixed 2.3.1 patch version of that package hasn't yet been released

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.

download pdf on custom button click
4 participants