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

The onChange prop is not updated on every render. #11

Closed
nicu-chiciuc opened this issue Apr 23, 2019 · 4 comments
Closed

The onChange prop is not updated on every render. #11

nicu-chiciuc opened this issue Apr 23, 2019 · 4 comments
Labels
bug Something isn't working

Comments

@nicu-chiciuc
Copy link

The onChange prop is passed to the font-picker constructor during the initialization of the component. This means that changing the onChange prop will not change the callback that was passed initially.
Since I'm using hooks for my component this means that no matter what changes, the FontPicker will always call the initially passed callback.

@samuelmeuli samuelmeuli added the bug Something isn't working label May 12, 2019
@samuelmeuli
Copy link
Owner

Hi @nicu-chiciuc, thanks for letting me know about this problem.

You're right, the onChange prop is currently not reactive, the docs on this were incorrect. Sorry about that. I'd argue that you don't normally need it to be, though. Could you give me a possible use case?

I've implemented the project's examples with hooks as well (see this file). Maybe it wasn't working for you because of this bug. Could you please let me know if you're still encountering problems with the latest release?

@nicu-chiciuc
Copy link
Author

Sorry for the very late reply. I've solved the issue by sending a function which calls another function that is updated using a ref. The problem with your hooks example is that the passed anonymous function passed to onChange will be the first one that was created during initialization.

// HACK: The FontPicker component will not change the `onChange` prop
  if (setFontRef) {
    setFontRef.current = setFontFamilyCurrent;
  }
  const setFontFamilyInitial = (newFont: { family: string }) => {
    if (setFontRef && setFontRef.current) {
      setFontRef.current(newFont);
    }
  };

The setFontFamilyCurrent function will be the one that is updated on each rerender.

@samuelmeuli samuelmeuli reopened this May 29, 2019
samuelmeuli added a commit to samuelmeuli/font-picker that referenced this issue May 29, 2019
@samuelmeuli
Copy link
Owner

Thanks for your response. I've just made the onChange prop reactive. Your code should work without the workaround with the new release.

@nicu-chiciuc
Copy link
Author

Thank you @samuelmeuli

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants