Skip to content

Conversation

@leimer
Copy link

@leimer leimer commented May 7, 2024

Thanks for your highly appreciated work on the label template. I like it a lot :-) Thank you very much!

I came to notice the script is using an online service to render QR codes.
I'd like to suggest to use a simple and stable javascript library to generate the QR codes locally without any additional network traffic. This website - once stored locally - can then be used at offline locations.

@leimer
Copy link
Author

leimer commented May 10, 2024

Just realized: the codes get only rendered once. Feels like the x-init attribute is executed only once. I will update my PR. Please stay tuned ;-)

@tmaier
Copy link
Owner

tmaier commented May 26, 2024

Hi, thanks for your contribution.

You are using a very dated library. One which has not been updated for 9 years. Please consider replacing it with a more popular solution.

index.html Outdated
width="100"
height="100"
<div
x-effect="$el.innerHTML=''; new QRCode($el, { text: label.text, correctLevel: QRCode.CorrectLevel.L});"
Copy link
Owner

Choose a reason for hiding this comment

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

I think instead of $el.innerHTML='' , one should call qrcode.clear()

I think one could also call qrcode.makeCode("...");

This could mean we could put all this code down into a dedicated method within qrCodeApp

@leimer
Copy link
Author

leimer commented May 26, 2024

You are using a very dated library. One which has not been updated for 9 years. Please consider replacing it with a more popular solution.

I tried switching to qrcode, but it loads dependencies by require, which is unavailable inside the browser.
I found less popular but newer QRious library, which works even smoother. Feels like the qrcode libraries are pretty stable.

Since this is a very simple tool, with only local execution, I don't have the normal paranoia regarding updated libs.

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