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

Feature/more controls over custom trigger #244

Conversation

vtsybulin
Copy link
Contributor

@vtsybulin vtsybulin commented May 5, 2020

@gregnb @MatthewHerbst Hello! Faced kinda same need as per #219

However I need even more control then just binding to some single trigger ref, so I decided to go with context provider to expose the handleClick method directly to be used afterward. Sample:

import ReactToPrint, { PrintContext } from 'react-to-print';

...

<ReactToPrint content={() => this.componentRef}>
    <PrintContext.Consumer>
	{({ handlePrint }) => (
          <button onClick={handlePrint}>Print this out!</button>
	)}
     </PrintContext.Consumer>
</ReactToPrint>

Also wrapped ReactToPrint into [useReactToPrint] hook to be used with functional components. Sample:

import { useReactToPrint } from 'react-to-print';

...

const Example = () => {
  const componentRef = useRef();
  const handlePrint = useReactToPrint({
    content: () => componentRef.current,
  });
  
  return (
    <div>
      <ComponentToPrint ref={componentRef} />
      <button onClick={handlePrint}>Print this out!</button>
    </div>
  );
};

Let me know what you think! Thanks.

Copy link
Owner

@MatthewHerbst MatthewHerbst left a comment

Choose a reason for hiding this comment

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

This is fantastic, thank you!

My only concern is that it would be a breaking change since we currently support React 15. As mentioned in the review comments, I don't mind making that breaking change, but would want to add a few more breaking changes in at the same time which could delay this by a couple of days. Do you mind waiting until the weekend for this to be released? Happy to get it out sooner if you can make the code not break when using React 15.

README.md Outdated
@@ -51,6 +51,13 @@ The component accepts the following props:
| **`bodyClass`** | `string?` | Class to pass to the print window body |
| **`suppressErrors`** | `boolean?` | When passed, prevents `console` logging of errors

### PrintContext

In case you don't want to specify `trigger` directly within other props but you rather want to gain some extra control over it, you could make use of PrintContext. By using PrintContext you can get direct access to the `handlePrint` method which triggers printing action.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
In case you don't want to specify `trigger` directly within other props but you rather want to gain some extra control over it, you could make use of PrintContext. By using PrintContext you can get direct access to the `handlePrint` method which triggers printing action.
If you need extra control over printing and don't want to specify `trigger` directly, `PrintContext` allows you to gain direct access to the `handlePrint` method which triggers the print action.

README.md Outdated
In case you don't want to specify `trigger` directly within other props but you rather want to gain some extra control over it, you could make use of PrintContext. By using PrintContext you can get direct access to the `handlePrint` method which triggers printing action.

### [useReactToPrint]
For the functional components, there is a useReactToPrint hook, which accepts an object with the same configuration props as `<ReactToPrint />` itself and returns a handlePrint function, by calling which you can trigger printing action.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
For the functional components, there is a useReactToPrint hook, which accepts an object with the same configuration props as `<ReactToPrint />` itself and returns a handlePrint function, by calling which you can trigger printing action.
For functional components, use the `useReactToPrint` hook, which accepts an object with the same configuration props as `<ReactToPrint />` returns a `handlePrint` function which when called will trigger the print action.

@@ -135,6 +189,48 @@ const Example = () => {
</div>
);
};

```
### Calling from functional components with [useReactToPrint](https://reactjs.org/docs/hooks-intro.html)
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
### Calling from functional components with [useReactToPrint](https://reactjs.org/docs/hooks-intro.html)
### Calling from functional components with `useReactToPrint` [hook](https://reactjs.org/docs/hooks-intro.html)

src/index.tsx Outdated
export interface IPrintContextProps {
handlePrint: () => void,
};
export const PrintContext = React.createContext({} as IPrintContextProps);
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a way to make this conditional based on React.createContext existing? Right now we support React v15 which does not have createContext (introduced in React 16.3.0). I would actually be ok making a major version change to drop React 15, however, I'd want to get a few more breaking changes in I've been meaning to do, which would delay shipping this by a few days.

src/index.tsx Outdated
}
}

export const useReactToPrint = (options: IReactToPrintProps) => {
Copy link
Owner

Choose a reason for hiding this comment

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

Similar to the above comment, is there a way to make this a no-op (just export undefined or something) if the user is on React 15 (which doesn't have useMemo and useCallback?

@vtsybulin
Copy link
Contributor Author

@MatthewHerbst Hi. Thanks for the feedback! Sounds cool. Sorry for the v15 thing missed that :/ Let me take a look tomorrow so I can update accordingly If you're ok with that?

@MatthewHerbst
Copy link
Owner

No problem. If you're not in a rush then I'm happy to move deploying this to the weekend so I can get a few other changes in, and then we can do a major version release and drop React 15 support (which we should do anyways for a variety of reasons).

@jtart
Copy link

jtart commented May 15, 2020

hey @MatthewHerbst, are you close to releasing this at all?

@vtsybulin
Copy link
Contributor Author

@MatthewHerbst Hey! Sorry, it's been a while, was a bit busy. I hope it's better now. Let me know if anything extra is required, thanks!

@MatthewHerbst MatthewHerbst merged commit c26fd00 into MatthewHerbst:master May 16, 2020
@vtsybulin
Copy link
Contributor Author

@MatthewHerbst nice! thanks 👍

@MatthewHerbst
Copy link
Owner

@vtsybulin thank you! I'm working on a few small things, will have it in a new release in a couple of hours

@vtsybulin
Copy link
Contributor Author

@MatthewHerbst Cool. Let me know if you have anything else in mind to assist with.

@MatthewHerbst
Copy link
Owner

This is published with the just released version 2.8.0-beta.1. I'll release 2.8.0 either later tonight or tomorrow once I have a chance to do more testing.

@MatthewHerbst
Copy link
Owner

Published as 2.8.0, thanks again!

@vtsybulin
Copy link
Contributor Author

Great 👍

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

Successfully merging this pull request may close these issues.

3 participants