Barcode QRcode #16015
Barcode QRcode #16015
Conversation
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
packages/client/package.json
Outdated
| @@ -35,7 +35,10 @@ | |||
| "sanitize-html": "^2.13.0", | |||
| "screenfull": "^6.0.1", | |||
| "shortid": "^2.2.15", | |||
| "svelte-spa-router": "^4.0.1" | |||
| "svelte-spa-router": "^4.0.1", | |||
| "atrament": "^4.3.0", | |||
There was a problem hiding this comment.
atrament doesn't seem to be used, so can be removed.
| import { onMount, afterUpdate } from "svelte" | ||
| import JsBarcode from "jsbarcode" | ||
| import { createQrSvgString } from "@svelte-put/qr" | ||
| import "@spectrum-css/vars/dist/spectrum-global.css" |
There was a problem hiding this comment.
You shouldn't need to manually import spectrum CSS - the client library already does this.
| export let customLogo: string | ||
| export let size: number | ||
| export let primColor: string | ||
| export let vertical: boolean |
There was a problem hiding this comment.
None of these settings are required in the manifest, so the types should account for the fact they may be undefined. The boolean settings are fine as long as you have defaultValue in the manifest (which you do). But for the strings and number setting you'll need to type those as string | undefined and handle that as needed.
| </div> | ||
| {:else} | ||
| <div id="barcode-container"> | ||
| <div id="logo-and-barcode" style="height: {size}px, width: 100%"> |
There was a problem hiding this comment.
I can see that you're using quite a lot of ids here. You should never use the same ID more than once on a page, but adding multiple of this component will create multiple elements with the same ID.
The 2 options here are:
- Generate a unique ID for each instance of the component. This is harder to style because you'll need to get this ID into your styles section, so I don't recomment this approach.
- Don't use ID attributes at all. You can style elements using classes, and you can get the reference to your DOM element for jsbarcode by using binding, like you do with the qrContainer. I checked the source and it looks like it supports passing a DOM element in as the parameter instead of a string selector.
There was a problem hiding this comment.
I switched the way in which the barcode element is created, meaning it no long requires an element-ID to target. The rest of the IDs have been replaced.
| generateBarcode() | ||
| } else { | ||
| generateQr() | ||
| } |
There was a problem hiding this comment.
Could you just double check that this can't cause an endless loop? afterUpdate fires after any component DOM update, and you're mutating the DOM inside the callback, which smells a little risky. I'd also just double check that this does properly react to all settings changes as expected - e.g. try changing all of these props at runtime and check the component redraws the barcode as expected without needing a page refresh.
Basically if you could just add a console log or something, and ensure it's not constantly firing! And just check both barcode and QR code to be sure.
The usual way to handle this would be doing a (rather large) reactive statement and passing in all the required props that we'd want to observe and re-render the barcode when they change, but this is fine too as long as it doesn't cause too many wasted invocation.
There was a problem hiding this comment.
This was an interesting one - updating a setting triggered off a handful of console.logs()
Description
This PR adds a new component to display QR Codes and Barcodes based on the value passed. This could be a static value, or one passed in using bindings/JS.
This component also has options such as "Display Value"
And also "Show logo"
QR Codes can also select their primary color
Barcode/QR Code Generators can also be used in repeaters
Barcode/QR Code Generator in PDF screens
Addresses
<Enter the Link to the issue(s) this PR addresses>App Export
Barcode Gen-export-1745587460313.tar.gz
Screenshots
If a UI facing feature, a short video of the happy path, and some screenshots of the new functionality.
Launchcontrol
Adds Barcode/QR Code generator.