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

fix(258): improve useEffect call #259

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

spaudanjo
Copy link

  • add missing dependencies
  • add additional effect which stops the scanner when the component unmounts

Closes #258

HINT:
I only tested this roughly on my Chrome (Mac) and Chrome (Android) the new expected behaviour works. Not sure about potential breaking changes or performance drops (e.g. because of missing stop calls to previous versions of BrowserQRCodeReader).

* add missing dependencies
* add additional effect which stops the scanner when the component unmounts
@spaudanjo
Copy link
Author

Please let me know your thoughts on this @JodusNodus :)

@Corneliuus
Copy link

Can we have this merged into master please? @JodusNodus

@victorrseloy
Copy link

it would be pretty nice to have this merged @JodusNodus.

@Mistifiou
Copy link

Mistifiou commented Aug 11, 2022

Hi,

with this commit and this update on file I was able to fix #258 for my case.

      codeReader
        .decodeFromConstraints({ video }, videoId, (result, error, controls) => {
          controlsRef.current = controls
          if (isValidType(onResult, 'onResult', 'function')) {
            onResult(result, error, codeReader, controls);
          }
        })
        .catch((error: Error) => {
          if (isValidType(onResult, 'onResult', 'function')) {
            onResult(null, error, codeReader, controlsRef.current);
          }
        });
    }

@JayWelsh
Copy link

Hi @spaudanjo , I was wondering if you have any thoughts about your method of stopping the camera on mount vs the method used here?

#279

I've merged the linked PR into my fork ( https://github.com/JayWelsh/react-qr-reader ), and would like to merge more improvements into my fork (such as your dependency enhancement) which is why I am asking.

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