search

nimiq / qr-scanner

Lightweight Javascript QR Code Scanner
https://nimiq.github.io/qr-scanner/demo
MIT License
2.36k stars 511 forks source link

Body append #129

Open VincentGillot opened 2 years ago

VincentGillot commented 2 years ago

I was wondering why is the video re-appended to the body? I'm using this inside a React app and it's being put outside of the screen. It would be nice for it to just simply use the current video tag or choose a parent element to be used for appending. And i can't style it because all the styles are being overwritten by the instance construction.

danimoh commented 2 years ago

Hello, the video is not being re-appended to the body, it is only being appended to the body, if it is not appended to the DOM yet at the time the QrScanner instance is created. This is to avoid issues with iOS which disallows playing a video that's not in the DOM or hidden, see https://github.com/nimiq/qr-scanner/blob/8bb46c3cbc6e6b1c256a7ef6abc866d734493f36/src/qr-scanner.ts#L177-L187 https://github.com/nimiq/qr-scanner/blob/8bb46c3cbc6e6b1c256a7ef6abc866d734493f36/src/qr-scanner.ts#L229-L258

You can avoid this by creating the QrScanner instance only as soon as the video element has been added to the DOM. If you're using a UI framework, note that most frameworks offer something like a mounted hook, that tells you that a component's DOM elements have actually been added to the DOM.

Instead of doing the checks linked above in the constructor, I could also do them on each play(). Do you think that would help?

brettwillis commented 2 years ago

@danimoh this line prevents us from showing the video inside the shadow DOM. Can't use it in UI frameworks that utilise shadow DOM (web components) e.g. LitElement.

https://github.com/nimiq/qr-scanner/blob/34bccc6b278672e28d6eb62f07c1832f1e6d2e92/src/qr-scanner.ts#L185

Edit: this particular problem is already raised by https://github.com/nimiq/qr-scanner/issues/168.

webermax commented 1 year ago

For our project, that's a show stopper, too.

exetico commented 1 year ago

What's the progress on this? It's a show-stopper here, too (Lit are in use, indeed).

Have you done any work behind the scenes, @nimiq ? Thank you for a great solution for qr-scanner.

exetico commented 1 year ago

@webermax Now we just need @nimiq to approve https://github.com/nimiq/qr-scanner/pull/241. You're welcome to use my fork, while we wait.

https://github.com/exetico/qr-scanner/tree/master

I'll remove my work, once the merge has (hopefully) been approved.