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

Update html5-qrcode-scanner.ts #431

Closed
wants to merge 3 commits into from
Closed

Conversation

Joggel72
Copy link
Contributor

Add attribute type with value button to button-tags, so Firefox will not do a submit as discussed in comment #413 (comment)

Add attribute type with value button to button-tags, so Firefox will not do a submit
README.md Outdated
@@ -1,856 +1,4 @@
# Html5-QRCode
## (supports barcodes now :))
A cross-platform HTML5 QR code & barcode reader.
Fork for pull request for mebjas/html5-qrcode
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please revert this change.

@@ -492,6 +492,7 @@ export class Html5QrcodeScanner {
requestPermissionContainer: HTMLDivElement) {
const $this = this;
const requestPermissionButton = document.createElement("button");
requestPermissionButton.setAttribute("type", "button");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, fix the indent.

@@ -640,11 +641,13 @@ export class Html5QrcodeScanner {

const cameraActionContainer = document.createElement("span");
const cameraActionStartButton = document.createElement("button");
cameraActionStartButton.setAttribute("type", "button");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we define a method

private createButton(): HtmlButtonElement {
    let button = document.createElement("button");
    button.setAttribute("type", "button");
    return button;
}

And use it in all these places?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that would be a nice solution to get every button at the same implementation state.

@mebjas
Copy link
Owner

mebjas commented Feb 26, 2022

Thanks for the PR, please take a look at the review comments.

@mebjas
Copy link
Owner

mebjas commented Mar 4, 2022

Hi @Joggel72 - Please revert back the readme and then we can submit.

@Joggel72
Copy link
Contributor Author

Joggel72 commented Mar 9, 2022

Hi @Joggel72 - Please revert back the readme and then we can submit.

I hope I've made it correct by deleting the file.

@mebjas mebjas mentioned this pull request Nov 12, 2022
@mebjas
Copy link
Owner

mebjas commented Nov 12, 2022

Thanks for the PR, the PR is fairly outdated.

Will send another PR with your commit (you as author) - #588

Appreciate the contribution. Closing this one.

@mebjas mebjas closed this Nov 12, 2022
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

Successfully merging this pull request may close these issues.

2 participants