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

UpdatedDom ignored on onbeforeprint #140

Closed
LeopoldLerch opened this issue Jun 28, 2019 · 24 comments
Closed

UpdatedDom ignored on onbeforeprint #140

LeopoldLerch opened this issue Jun 28, 2019 · 24 comments

Comments

@LeopoldLerch
Copy link

LeopoldLerch commented Jun 28, 2019

Hi, I am using this lines

<ReactToPrint
  trigger={() => <a href="#">Print this out!</a>}
  content={() => componentRef.current}
  onBeforePrint={() => setPrintMode(true)}
  onAfterPrint={() => setPrintMode(false)}
/>

where i am setting Printmode to true, in order to render additional Content (Legend) for the print.

{printMode &&
  <div>
    {data.legende.map((l) => <div>{`${l.abk}: ${l.text}`}</div>)}
  </div>
}

However. It is called and the DOM updated (as i can see in the background, but in the print window it still shows the "non-print-version".

the full thing I use:

const componentRef = useRef();

return (
  <div className={css(styles.formsContainer)} ref={componentRef} >
    <h2 className={styles.formsTitle}>
      {Strings.PressespiegelHeaderText}
      {!printMode &&
        <ReactToPrint
          trigger={() => <a href="#">Print this out!</a>}
          content={() => componentRef.current}
          onBeforePrint={() => setPrintMode(true)}
          onAfterPrint={() => setPrintMode(false)}
        />
      }
      </h2>
      {data &&
        <div>
          {`Ausgabe vom ${data.datum}`}
          {data.kapitel.slice(1).map((kapitel) => {
            return (
              <div key={kapitel.id}>
                <div>
                  {kapitel.bezeichnung}
                </div>
                <ListBase
                  hideArrow
                  items={kapitel.KM}
                  renderItem={renderKM}
                />
              </div>
            );
          })}
          {printMode &&
            <div>
              {data.legende.map((l) => <div>{`${l.abk}: ${l.text}`}</div>)}
            </div>}
            {data.copyright}
        </div>
      }
    </div>
);`
@MatthewHerbst
Copy link
Owner

Hello. Can you please show what setPrintMode does?

@LeopoldLerch
Copy link
Author

LeopoldLerch commented Jun 28, 2019 via email

@MatthewHerbst
Copy link
Owner

Hmm. setState is an asynchronous function, but onBeforePrint is currently setup to be a synchronous function. Essentially, react-to-print doesn't know that it needs to wait for some async work to finish. This is clearly an oversight. Let me see if I can come up with a quick solution that won't break anyone else.

@LeopoldLerch
Copy link
Author

Thank you. Well, in React I can imagine many cases when I need to "react" to "beforePrint" by setting states. So I think this might be an issue others are also running into.

@MatthewHerbst
Copy link
Owner

I'm trying to think if there is an elegant way to do this without using Promise. I like Promise, but it is not supported by IE and other older browsers that we currently work on. If there's no other good way to do this maybe it's time we start making people use a polyfill, but I'll have to see what the community thinks in that case.

@LeopoldLerch
Copy link
Author

I see. My opinion is, that we should use new technologies if they fit us. Holding them back to support outdated browsers is just slowing down development at all (in fact also for npm-packages).
As in our current project (which heavily relies on fetching data) we still support IE, but we polyfill everything it does not support but keep using modern technologies.

If a solution wants to use reat-to-print, it might have polyfilled for IE anyways, as a client-application in most cases uses eg fetch, array-functions, ... .

Pls keep me updated. But for the moment i guess I won´t be able to change anything on "onbeforeprint". Thought about it, but I didn´t came up with a use-case for "onbeforeprint", that would not resolve in a change in the state of a component (if I leave direct DOM-Manipulation out as it is not recommended in React)

@MatthewHerbst
Copy link
Owner

I'm very open to using promises and making people polyfill for support. Just will be a slightly longer process since it involves releasing a breaking change. Give me a couple of days to work on this - I do think it's needed, so thank you for the idea!

@MatthewHerbst
Copy link
Owner

So, the way I envision using this is that onBeforePrint, if given, can return a Promise. react-to-print will wait for that promise to resolve/reject before attempting to print. With your specific use case in mind, you would have to wrap your setState call in a Promise so that we could track it. So, you would have to do something like:

// This is the function you would pass to `onBeforePrint`
handleBeforePrint = () => {
  return new Promise((resolve, reject) => {
    this.setState(newState, resolve);
  });
}

Does that seem ok to you?

@MatthewHerbst
Copy link
Owner

@gregnb do you have any objection to anything in this thread?

@LeopoldLerch
Copy link
Author

So, the way I envision using this is that onBeforePrint, if given, can return a Promise. react-to-print will wait for that promise to resolve/reject before attempting to print. With your specific use case in mind, you would have to wrap your setState call in a Promise so that we could track it. So, you would have to do something like:

// This is the function you would pass to `onBeforePrint`
handleBeforePrint = () => {
  return new Promise((resolve, reject) => {
    this.setState(newState, resolve);
  });
}

Does that seem ok to you?

might work out. One has to try if the rendering is already done too, when the setState calls the callback function.

My current situation is based on hooks. As I know hooks don´t have such a callback option. Do you have any suggestion in that case too?

@MatthewHerbst
Copy link
Owner

Hmm. I would image then that you would have to save a reference to the resolve and reject functions and call them when needed.

So, in a class world:

// This is the function you would pass to `onBeforePrint`
handleBeforePrint = () => {
  return new Promise((resolve, reject) => {
    this.resolve = resolve;
    this.reject = reject;
    this.setState(newState);
  });
}

componentDidCatch(error) {
  // You would likely want to check that the update was for the `setState` you called versus just a prop change or something
  this.reject();
}

componentDidUpdate() {
  // You would likely want to check that the update was for the `setState` you called versus just a prop change or something. Could do that by setting a special key in state or something
  this.resolve();
}

Functional could work in a somewhat similar manner. (This is a quick example that I thought about that I think will work, and likely could be improved.)

const Component = (props) => {
  const [state, setState] = useState({ printMode: false });

  // We could talk about allowing `onAfterPrint` to handle Promises as well but I don't think there is a use case for that right now?
  const handleAfterPrint = useCallback(
    () => {
      const { resolve, reject, ...rest } = state;
      setState({ ...rest, printMode: false });
    },
    [state] // Micro-optimization, the `useCallback` likely isn't needed
  );

  const handleBeforePrint = useCallback(
    () => {
      return new Promise((resolve, reject) => {
        setState({ ...state, printMode: true, resolve, reject })
      });
    },
    [state] // Micro-optimization, the `useCallback` likely isn't needed
  );

  // Runs _after_ the render has been completed
  useEffect(
    () => {
      const { resolve, reject, ...rest } = state;

      // Not sure if you would ever need to call `reject` since I don't think `setState` calls can fail?
      if (state.printMode && resolve) {
        resolve();
        setState(rest);
      }
    },
    [state]
  )

  return (
    <ReactToPrint
      ...
      onAfterPrint={handleAfterPrint}
      onBeforePrint={handleBeforePrint}
    />
  );
}

What do you think about that?

Thanks again for the discussion on this, very interesting to work through!

@LeopoldLerch
Copy link
Author

Hmm. Looks quite complicated and error prone. I don´t have much time to try out too.

Currently I am thinking about a React-context could help us out. I don´t know if this is async too.

@MatthewHerbst
Copy link
Owner

If you can think of a better solution please let me know. The issue is two fold: the parent has to resolve/reject the promise, and only the parent knows when it is done re-rendering.

@michael-ecb
Copy link

I have the same issue, I want to render only when print clicked,
for now I 'm using this solution but sometimes not works ...

 <ReactToPrint
            trigger={() =><span><IconButton onClick={this.setPrintMode(true)}><PrintIcon/></IconButton></span>}
            content={() => this._ref.current}
            onAfterPrint={this.setPrintMode(false)}
            bodyClass={"print"}/>
            <div data-printable="" ref={this._ref}>
                {print && children}
            </div>

@MatthewHerbst
Copy link
Owner

@michael-ecb it sometimes works because there is a race condition happening, so sometimes your render will beat it and sometimes it won't. What are your thoughts on what I've suggested for how it could be done above?

@michael-ecb
Copy link

@MatthewHerbst sounds greater than use React-context ...

@MatthewHerbst
Copy link
Owner

We could look into using context, though it would likely require us to raise our minimum required React version, which I think is a downside compared to asking people to provide a Promise polyfill since most probably already do that if they support IE and such.

@LeopoldLerch
Copy link
Author

Hmm.
Maybe a MutationObserver would be a way to Go?

@MatthewHerbst
Copy link
Owner

Interesting, I didn't know about the API before. Can you maybe write a pseudo-code example of how you think that would work? From the docs I don't see any way of the observer telling you that the mutation is done, rather, you have to tell it that the mutation is done. We could pass a function from props down into the callback, but that would require users of the library to have intimate knowledge of how MutationObserver works, which I don't think it something we should assume.

@LeopoldLerch
Copy link
Author

Well, it tells you if something in DOM changed.

Basically you create an observer-object which includes the callback. and thell it to observe a dom-element with given extra-config-parameters. (plain js as here: https://medium.com/@abbeal/listening-to-the-dom-changes-with-mutationobserver-b53a068a58d2)

You can even use a existing react-component like
https://github.com/jcgertig/react-mutation-observer

I thought of something like this:
As "our" components are given to react-to-print-component as a child, react-to-print could wrap the children prop internally into that mutationobserver. That way it could handle itself to go on with printing after it recognizes (via the mutationobserver-callbacks) that the dom inside it (which includes "our" components had changed. That way the users would not have to know anything about a mutationobserver.

However, mutationobserver is IE11 and above.

and also, it might also not work in our case as i think of it. As it would recognize changes, but it would trigger after the first change (which might not be enough) and would never trigger when react doesn´t update the DOM at all.

@LeopoldLerch
Copy link
Author

LeopoldLerch commented Jul 17, 2019

I think there are only 2 ways:

  1. the child provides a callback function for beforeprint, that gives a promise, which is then approved from the child itself when it is done (as only the child can know when it is finished)
  2. using react-Context (i know, again): first, it is providing it's own state (eg PrintMode) via a context-provider, in which the "users"-component given as children-property is wrapped in inside the render-method. When clicking "Print", that state is set to true. This in turn triggers rerendering the react-to-print-component as well as the given "users"-component inside it. As I know of react, the render-method is finished, when it and all components inside have finished called their render-functions. And, after a component is rerendered, the componentDidUpdate method is called. There we can hook in. That componentdidupdate then checks if the react-to-print's state is "printmode", and then it executes the whole stuff for printing (copying the html-stuff and then call the window.print on that). After that, it will revert the state to print-mode=false, which will reset everything to "normal"-layout.

Hmm, second way has the same drawback like MutationObserver in terms of reacting on the very first rerendering, however, this might be enough at all. Additionally, second method uses already-built-in react-features, doesn't need a promise and is quite easy to use for users. (as they would only need to wrap their render-function into a tag.

@LeopoldLerch
Copy link
Author

Ok, react-to-print doesn´t use a children-prop. Don´t know where i got that from. However, Context-method would work also, altough a bit less easy
React-to-print could provide an interface how an provider should look like. The user wraps React-to-print and the child-component inside such a provider. from there upwards it is all the same like above. Via a function react-to-print sets the printmode-value of the privider to true, which in turn triggers then rerendering all of the components that have a consumer of that provider (which is at least react-to-print). in componentdidupdate, react to print then executes the whole printing-stuff (not even that onbefore-print-callback is needed). and then resets that printmode-value of the provider which resets everything again

Sorry for that much text. I will be able to come up with some code-examples when i work again on that part of my project which uses react-to-print. But that will be in ~ 3 weeks i guess as i am on holiday. Currently I just can share my thoughts.

@MatthewHerbst
Copy link
Owner

MatthewHerbst commented Jul 17, 2019

I personally think the simplest solution is for onBeforePrint to optionally return a Promise that we will wait to be resolved before printing. That way the user can decide however they would like when/how to resolve the Promise.

The proposal you've suggested with waiting for componentDidUpdate to run won't work because it won't have any knowledge of async calls within the lifecycle methods of child components. For example, if a child component renders some temporary state and makes a network call, and then will re-render when the network call finishes, componentDidUpdate will still fire because it does not know about the pending Promise.

Promise is a widely supported library that just about everyone doing JS these days should be knowledgeable with. Other than requiring a browser polyfill for older browsers (which I would guess most users are already doing if they support those browsers), I don't see any downside to it.

@MatthewHerbst
Copy link
Owner

This has been addressed in #146, published as v2.2.0 which adds support for onBeforePrint to return a Promise. Please let me know if you encounter any issues

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

No branches or pull requests

3 participants