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

Clear image from file input #193

Closed
ROBERT-MCDOWELL opened this issue Apr 26, 2021 · 10 comments · Fixed by #371
Closed

Clear image from file input #193

ROBERT-MCDOWELL opened this issue Apr 26, 2021 · 10 comments · Fixed by #371

Comments

@ROBERT-MCDOWELL
Copy link

Describe the bug
from Html5QrcodeScanner class
clear function does not remove the loaded file with file upload mode

To Reproduce
load an image from file upload input
use Html5QrcodeScanner.clear();

Expected behavior
clear must clear all current variables and remove the image canvas showing the file loaded

Smartphone (please complete the following information):

  • Device: a;ll
  • OS: all
  • Browser all
  • Version last
@fssrepository
Copy link

fssrepository commented Apr 26, 2021

I also want to mention related to it, that with "stop" i don't want to clear the last image from the canvas, just with "clear" (I'm sending it to the backend)

Pls. remove line: 452 - $this._element.removeChild($this._canvasElement);
Just use on clear.

From 149 - 156 is not that good. Pls. use either promise or callback, but not both.

qrCodeErrorCallback - verbose flag is missing - i have been forced to define an empty function, to avoid logging.

@ROBERT-MCDOWELL
Copy link
Author

mmhm ok, I just realized also that the clear() function remove completely the scanner if it's currently scanning,
but not effect on IDLE and with image input upload file loaded.
I thought this clear function was to reset all the data loaded with the instance. maybe the right name for this actual function would be "remove", but it then needs to remove the scanner also with idle status.

@ROBERT-MCDOWELL
Copy link
Author

Ok I think I have to make a PR, the code is incomplete and using "clear" to "remove" is confusing.
also the "clear" function does not include any upload file input panel as it states that only if the scanner is scanning so it can be cleared....

@ROBERT-MCDOWELL
Copy link
Author

do you know how to use the sources rather than the minified file? tried to import html5-qrcode.js html5-qrcode-scanner.js and zxing-js.umd.min.js but does not work. My problem is for my test I don't want to bother to minify everytime, and also there is no minified on Fedora 33 but yui-compressor which destroy the current code...

@ROBERT-MCDOWELL
Copy link
Author

ok for now I mad a temporary trick to clear the canvas + file input;

var canvas = document.getElementById("qr-canvas-visible");
var file = document.getElementById("[scannerContainerName]__filescan_input");
var link = document.getElementById("[scannerContainerName]__dashboard_section_swaplink");
scannerInstance.currentScanType = (scannerInstance.currentScanType == Html5QrcodeScanner.SCAN_TYPE_CAMERA) ? Html5QrcodeScanner.SCAN_TYPE_FILE : Html5QrcodeScanner.SCAN_TYPE_CAMERA;
if(canvas){
var context = canvas.getContext("2d");
context.clearRect(0,0,canvas.width,canvas.height);
}
if(file){
file.value = "";
}
document.getElementById("[scannerContainerName]__dashboard_section_swaplink").dispatchEvent(new Event('click',{'view':window,'bubbles':true,'cancelable':true}));

@fssrepository
Copy link

fssrepository commented Apr 27, 2021

Robert, if you have time, it's not that difficult to rewrite the whole, i don't think it's good enough for production, little bit spaghetti here and there. Unfortunately, i'm working with angular, and with typescript, so even if i refactor it completely it won't be javascript.
(my version is generating qrcode also - very likely i prefer to use the same canvas, what this component is creating)

I might also reduce the number of parameters to make it cleaner. I have an image upload, where the user can rotate / scale the image, i'm not sure what the flip is all about.

Anyway Shape Recognition API on the way for chrome at least, so for the next coming month Zing might not be needed anymore. Next year this library might be completely obsolete, when other browsers are introducing the API also. (text/face recognition is included also) Native QR Code scanner already in Canary.

@ROBERT-MCDOWELL
Copy link
Author

I unfortunately no time to pass to much on html5-qrcode project, so as long as my spaghettis work for old and new browsers it's ok.
I need to support old browsers, so use the native coming browser is not a good solution. Also I really don't trust browsers anymore especially if they introduce face recognition and other gadgets that will certainly open doors to illegal tracing and private data capture.

@mebjas
Copy link
Owner

mebjas commented May 3, 2021

@ROBERT-MCDOWELL
Thanks for reporting the issue, I have been able to reproduce it (& some more bugs around it). I'll take it on, thanks for the patience.

@fssrepository

I also want to mention related to it, that with "stop" i don't want to clear the last image from the canvas, just with "clear" (I'm sending it to the backend)

Please file a separate issue, with this context in detail

qrCodeErrorCallback - verbose flag is missing - i have been forced to define an empty function, to avoid logging.

You mean here, the callback should report the verboseity flag initially set?

@mebjas mebjas self-assigned this May 3, 2021
@fssrepository
Copy link

fssrepository commented May 3, 2021

If you bombard the logger 10 times per second as default, if i set verbosity flag to false, i expect that you avoid logging under this circumstances, as it has performance effect also. (and very likely meaningless, as i can't track problems) Until the plugin caugth my qr code, 2000 lines has been logged and i was not able to read it in the logger. You missed the versobe flag out completely from it. The error might not be needed to be logged that frequently also, as it does not really help to track issues. (i need to use the chrome developer scripting feature for that in this case, which has been used to debug mouse movement step by step for example, which would be ridiculous testing method in such a simple plugin)

Other sentence quite easy to explain. When QR code has been successfully scanned you are removing the canvas at stopping, however the stop should only freeze the last QR image on the canvas, the clear method should clear the image from the canvas, and the destroy should remove the canvas from the html tree. I also argue that it's necessary, as most of the time, the whole thing will be warpped, so destroying the html is just simply removing the wrapper div. (yes you need to take care of your variables, but i do think there are approaches when you don't need to bother that kind of cleanup)

@mebjas mebjas added this to the Version 2.1.0 milestone May 28, 2021
@mebjas
Copy link
Owner

mebjas commented Dec 20, 2021

Starting version 2.1.5 calling html5qrcodeScanner.clear() will also clear the entire UI which is rendered based on file scan as well.

mebjas added a commit that referenced this issue Dec 20, 2021
Calling `Html5QrcodeScanner#clear()` will also clear the UI rendered due to image based scan. This should address [issue#193](#193)
@mebjas mebjas mentioned this issue Dec 20, 2021
mebjas added a commit that referenced this issue Dec 20, 2021
* Version 2.1.5

### Changelog

 - Changed behavior from throwing error in case `qrbox.width` or `qrbox` is larger
    than the width of the root element. In such cases the dimension will automatically
    be truncated to the size of root element and will throw a warning based on verbosity
    settings. This should address [issue#357](#357)

* Fix error when qrbox size is not set.

* fix codacy error

* Calling Html5QrcodeScanner#clear() will clear the entire UI

Calling `Html5QrcodeScanner#clear()` will also clear the UI rendered due to image based scan. This should address [issue#193](#193)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants