Adds scanner to scan SHC for inclusion in a SMART Health Link #4

sarahmcdougall · 2022-09-19T19:55:53Z

Summary

Allows the user to scan a SMART Health Card for inclusion in a SMART Health Link.

New Behavior

When using the app, the user has the option to scan a SHC by navigating to the “Scan” tab. The “Scan” tab uses the qr-scanner library to detect a SHC QR from the webcam. See the SHC Health Card Verifier as another example of the qr-scanner being used for SHC scanning.

Once scanned successfully, the user can create a new SHL that contains the contents of the SHC, and create a custom name for the dataset that gets created. After creating the SHL, the data will be available for inclusion in custom datasets/other SHLs. If the user chooses not to create a SHL from the SHC, the SHC data will not be made available for inclusion in a custom dataset - they will need to scan the QR again.

jmandel

Thanks so much for putting this functionality together! I've made some inline suggestions and questions, but looking at the functionality overall I wonder if there might be ways to streamline.

The "QR Data Provider" seems very closely tied to the scanner, since it only manages a single SHC's worth of QRs at a time. Rather than modeling this as app-wide state with its own local storage, it might be more straightforward to think about the scanner component as managing all of this state transiently, and then calling something like saveNewSHC(jws) (i.e. calling dispatch({ type: 'vaccine-add...`) on the top-level app to add the vaccine to the store from the scanner, rather than jumping straight into a "New Scanned Dataset" screen with only a single SHC in the "set".

In other words, consider a flow like:

Scan some SHCs into store
Create datasets using the existing tools

... rather than adding a new kind of "create dataset" screen.

Does this make sense? Please free to push back if not; I'm a bit bleary-eyed after a few days at the HL7 conference ;-)

Edit: Could also consider calling out to an external scanner app to clarify boundaries here. For example, https://github.com/jmandel/health-card-scanner-demo/blob/main/public/client.html was the interface I put together last time I wanted to "call a scanner".

jmandel · 2022-09-21T03:50:59Z

ui/src/QrDataProvider.tsx

+  switch (action.type) {
+    case actions.SET_QR_CODES: {
+      let newState: State = initialState;
+      localStorage.setItem('qrCodes', JSON.stringify(action.qrCodes));


It looks like the reducer here is producing a side effect by writing data to localStorage. It would be best to ensure the reducer is a pure function, from (state, action) => state.

It'd be good to think about how to manage storage decisions in one place, so scanned QRs can be persisted in the same way as datasets, etc. Consider useEffect to watch for changes to the state and write them out to localStorage.

jmandel · 2022-09-21T03:52:16Z

ui/src/QrDataProvider.tsx

+
+      if (action.qrCodes) {
+        newState.qrCodes = action.qrCodes;
+        newState.jws = '';


Why is this blank assignment here? It's reassigned two lines later, and it's allowed to be null.

jmandel · 2022-09-21T03:59:18Z

ui/src/QrDataProvider.tsx

+  }).join('');
+};
+
+const reducer = (state: any, action: any) => {


Can you ensure that the inputs/outputs are typed, rather than any? Use of any leads to some surprised in code below like

validationStatus: state.validationStatus,

-- this is surprising because "validationStatus" does not exist on the State interface.

jmandel · 2022-09-21T04:04:43Z

ui/src/QrDataProvider.tsx

+interface State {
+  qrCodes?: null | string[];
+  jws?: null | string;
+  setQrCodes: (arg: any) => any;


Ca you type these to match the instances you have in initialState? e.g

()=>null

jmandel · 2022-09-21T04:24:31Z

ui/src/QrDataProvider.tsx

+
+const useQrDataContext = () => useContext(QrDataContext);
+
+export { QrDataContext, QrDataProvider, useQrDataContext };


Have you considered exposing just a a useQrStore() function here, returning just the fields in value above and making this object available as something like scannedQrStore in existing app-level context in

vaxx.link/ui/src/App.tsx

Line 825 in e4a98ae

<Outlet context={{ store, dispatch, serverSyncer }} />

?

This would help avoid the ceremony of a standalone context provider and avoid the need to wrap routes in index.tsx, and would help make this store look like and sit next to the existing app store.

jmandel · 2022-09-21T04:28:40Z

ui/src/QrScan.tsx

+    * Create QrScanner instance using video element and specify result/error conditions
+    * @param {HTMLVideoElement} videoElement HTML video element
+    */
+    const createQrScanner = async (videoElement: any) => {


Same question re: any

jmandel · 2022-09-21T04:31:07Z

ui/src/QrScan.tsx

+  const videoRef = useRef<any>(null);
+  // Get user media when the page first renders, and feed into createQrScanner()
+  useEffect(() => {
+    const getUserMedia = async () => {


Can these two inner const functions be lifted out of useEffect and just called from the effect? It's a bit hard to follow what the effect is doing specifically. It's also good to avoid re-defining each time the effect runs.

jmandel · 2022-09-21T04:33:32Z

ui/src/QrScan.tsx

+    }
+  };
+
+  const handleError = useCallback(() => {


What is the purpose of useCallback here? Would handleError = () => navigate(... work ?

jmandel · 2022-09-21T04:36:58Z

ui/src/QrScan.tsx

+    }
+
+    return () => {
+      if (runningQrScanner.current) runningQrScanner.current.stop();


There's a standalone effect above that also tries to stop the scanner once, on mount. Can you help me understand when/why the scanner would be running at mount time if this cleanup code is in place?

I removed the standalone effect from above - that was leftover from code used in the SHC web verifier app to switch the camera stream for mobile devices. In certain cases, the qrscanner's video stream would get interrupted and throw a DOMException. That extra handling is not needed here since the camera stream does not change when scanning

jmandel · 2022-09-21T04:40:22Z

ui/src/QrDataProvider.tsx

+const getJws = (qrCodes: string[]) => {
+  return qrCodes.map((c) => {
+    const sliceIndex = c.lastIndexOf('/');
+  const rawPayload = c.slice(sliceIndex + 1);


Looks like an indentation issue here? Are you using a formatting tool?

sarahmcdougall · 2022-09-26T15:46:11Z

In other words, consider a flow like:

Scan some SHCs into store

Create datasets using the existing tools

... rather than adding a new kind of "create dataset" screen.

Good thinking here. I think that this is a better flow and makes good use of the existing code. I pushed changes that should allow the user to scan as many SHC's as desired without automatically redirecting to a new tab. With the use of dispatch for adding the new SHC's, I don't think we need the QrDataProvider anymore?

jmandel

Thanks -- this is a big improvement! A few suggestions from here...

Line 225:6: React Hook useEffect has a missing dependency: 'store.vaccines'. Either include it or remove the dependency array react-hooks/exhaustive-deps

Would be good to resolve this (in testing, just adding this to the array seems to work).

Workflow for data storage: right now, scanning a card and creating a SHL works, but as soon as you reload the page, the stored SHC is gone. This leads to console logging like

TODO: synchronize deleted SHCs to SHLs

because the synchronization code detects that the SHC has been removed from the data set. It'd be good to have a strategy for persisting scanned SHCs, perhaps alongside the default data set? Then on startup we could load all the default immunizations as well as all user-scanned immunizations, by adding a line near https://github.com/jmandel/vaxx.link/blob/main/ui/src/App.tsx#L788 ? Happy to explore other possibilities here too; just want to have a clear expectation about how scanned SHCs are managed.

UX question: it's kind of awkward to just navigate away from the scanner after scanning SHCs. Maybe the "successful" scan flow should present a choice like "Scan another" or "Done", and below the camera view there could be a "Cancel" button that navigates away? Again, I'm open to other ideas here, but some sort of "rails" or guided path would be good.

sarahmcdougall · 2022-09-29T20:17:48Z

It'd be good to have a strategy for persisting scanned SHCs, perhaps alongside the default data set? Then on startup we could load all the default immunizations as well as all user-scanned immunizations, by adding a line near https://github.com/jmandel/vaxx.link/blob/main/ui/src/App.tsx#L788 ? Happy to explore other possibilities here too; just want to have a clear expectation about how scanned SHCs are managed.

UX question: it's kind of awkward to just navigate away from the scanner after scanning SHCs. Maybe the "successful" scan flow should present a choice like "Scan another" or "Done", and below the camera view there could be a "Cancel" button that navigates away? Again, I'm open to other ideas here, but some sort of "rails" or guided path would be good.

I agree with both these points - I also noticed the need for more management of the scanned SHC's, and the flow for scanning would be greatly improved with some more buttons and guidance. I will be on vacation through the next week, but can start looking into these enhancements once I am back.

sarahmcdougall added 3 commits September 13, 2022 16:14

add scanner as separate tab

3c193c3

fix video styling for multi qr

c4f4018

add try/catch

89e5129

jmandel requested changes Sep 21, 2022

View reviewed changes

sarahmcdougall added 3 commits September 23, 2022 11:07

typing changes

ea0098c

typescript fixes, shc storage refactor

53e423a

typescript updates

6301580

sarahmcdougall requested a review from jmandel September 26, 2022 15:46

jmandel requested changes Sep 28, 2022

View reviewed changes

jmandel mentioned this pull request Sep 28, 2022

Explicit prompt to "Load Sample Data" #5

Open


		const useQrDataContext = () => useContext(QrDataContext);

		export { QrDataContext, QrDataProvider, useQrDataContext };

Uh oh!

Adds scanner to scan SHC for inclusion in a SMART Health Link #4

Are you sure you want to change the base?

Adds scanner to scan SHC for inclusion in a SMART Health Link #4

Uh oh!

Conversation

sarahmcdougall commented Sep 19, 2022

Summary

New Behavior

Uh oh!

jmandel left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarahmcdougall commented Sep 26, 2022

Uh oh!

jmandel left a comment

Choose a reason for hiding this comment

Uh oh!

sarahmcdougall commented Sep 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jmandel left a comment •

edited

Loading