bcgov / vc-authn-oidc

Apache License 2.0
119 stars 69 forks source link

Scanning QR Code with a regular provides a bcwallet link on mobile #572

Open Gavinok opened 2 weeks ago

Gavinok commented 2 weeks ago

This PR resolves #383

When the QR code is scanned and opened by a regular phone camera the user will now be provided an option to open a BC wallet deeplink.

2024-07-04_14-55-31

Gavinok commented 2 weeks ago

I also corrected the path to the wallet icon which previously failed to load

loneil commented 2 weeks ago

Sorry haven't followed along with requirements from this one so maybe already sorted, but wondering about UX for what's supposed to happen to the "camera page"

If I scan with my camera and go to the link I get image Then click on the button and it opens the wallet with the proof 👍

So then:

To really know what happened to the deep link proof it would have to subscribe to the socket/proof pair like the QR code. Or poll for it? Could get messy on the controller side if multiple pages now need to know about the proof callbacks?

Another option is just in-page disable the button and set a status banner/spinner/something just when the button is clicked?

I don't think I see a UX design in the things I have links to for how this page would be intended to behave so maybe not worth handling the above if there's a different UX intended. Could check with Kim next week.

Also think the blue button style would probably want to be matching what's on the other mobile deep link buttons (just the darker blue).

If it's ok to not handle updates on the camera page until later then it could go in as is but might want to get Emiliano's say.

loneil commented 2 weeks ago

Clip showing user actioning the same deep link by going back to browser: https://github.com/bcgov/vc-authn-oidc/assets/17445138/ab2f1e5e-4ac7-4411-b6e8-35b22cec4371

Gavinok commented 2 weeks ago

I'll reach out to Kim in regards to how we want this to work. As you mention I think more feedback makes sense and some of it isn't that hard though some would require a connection. I'll also split off the styling into a separate file so we can share it across the html files.

Gavinok commented 1 week ago

By default the page will now attempt to redirect mobile users directly to the BC wallet https://0x0.st/XMPT.mp4

esune commented 1 week ago

Sorry I am late to the party here, but have some concerns that may need a chat to clarify:

c.c.: @knguyenBC - so the conversation is not fragmented everywhere

knguyenBC commented 1 week ago

Given that if the person scanned the QR code with a regular camera and they have BC Wallet installed, the BC Wallet app would still open, I think the "Open with BC Wallet" would not make sense.

I think the more appropriate call-to-action would if the person could download a digital wallet (such as BC Wallet).

Please see this user flow: https://miro.com/app/board/uXjVK0XtU3c=/?moveToWidget=3458764594303727078&cot=14 Here's also a wireframe of a suggested screen: https://www.figma.com/design/8OzWNqLpmh9M9nR0ynTTTh/VC-AuthN?m=dev&node-id=0-1

The wording is a bit more simplified and a new graphic to better visualize a digital wallet.

esune commented 1 week ago

Given that if the person scanned the QR code with a regular camera and they have BC Wallet installed, the BC Wallet app would still open, I think the "Open with BC Wallet" would not make sense.

I think the more appropriate call-to-action would if the person could download a digital wallet (such as BC Wallet).

Please see this user flow: https://miro.com/app/board/uXjVK0XtU3c=/?moveToWidget=3458764594303727078&cot=14 Here's also a wireframe of a suggested screen: https://www.figma.com/design/8OzWNqLpmh9M9nR0ynTTTh/VC-AuthN?m=dev&node-id=0-1

The wording is a bit more simplified and a new graphic to better visualize a digital wallet.

That's what we had in place already - possibly minus the link to download BC Wallet which can be easily added. To clarify: if the person uses the regular camera app it will NOT open BC Wallet - it will lead them to the above page where they could learn how to install a wallet app.

knguyenBC commented 1 week ago

If a person is taken to the "Please scan with a digital wallet..." page ,regardless if they were to scan with a regular camera app with BC Wallet installed on their device or without BC Wallet installed on their device, I think its okay to focus more getting the BC Wallet installed.

I think there's a higher chance that a person without BC Wallet installed will try to scan the QR code with a regular camera app than a person with BC Wallet to scan the QR code.

esune commented 1 week ago

If a person is taken to the "Please scan with a digital wallet..." page ,regardless if they were to scan with a regular camera app with BC Wallet installed on their device or without BC Wallet installed on their device, I think its okay to focus more getting the BC Wallet installed.

I think there's a higher chance that a person without BC Wallet installed will try to scan the QR code with a regular camera app than a person with BC Wallet to scan the QR code.

I met with @Gavinok and have to amend my previous statement: I somehow missed the part in his video where it clearly shows that even scanning with the camera app it will trigger the deep link to BC Wallet and continue the authentication flow. I still think adding instructions on how to install BC Wallet (if they get to teh page) and a "back" button to "start over" might be better than trying to tap into the auth flow from there. Thoughts?

knguyenBC commented 1 week ago

Correct me if I'm wrong, when the person scans the QR code on the webpage, the QR code page doesn't change and the QR code is still scannable so I don't think a back button is necessary. Unless you mean the above webpage that people get to after scanning the QR code with their camera app? I think a back button wouldn't really be useful in that case either as where would the back button go? Unless they had "scanned" the qr code on their mobile device by tapping it? But that's unlikely as they would have seen the mobile view of VC-AuthN.

loneil commented 1 week ago

@knguyenBC do have the image source for the main image on this mockup so @Gavinok can put it on the page

image

knguyenBC commented 1 week ago

Digital wallet graphic

Gavinok commented 1 week ago

I have changed the howto page the the following 2024-07-10_11-51-44

The deep link will still be tried by default as shown in the previous video and the Download BC Wallet button will link to the corresponding app store depending on the users mobile OS

knguyenBC commented 1 week ago

Is it possible to keep the padding/spacing between elements and typography consistent? The figma file should have the font styles. Don't need to follow to a T but maybe use it as a guidance on what should be consistent to each other. Ex. The "Learn more..", "Download BC Wallet", "Don't have a digital wallet", "This QR code..." should be the same font size. Thanks!

Gavinok commented 1 week ago

Sure thing

On Wed, Jul 10, 2024, 12:42 p.m. Kim Nguyen @.***> wrote:

Is it possible to keep the padding/spacing between elements and typography consistent? The figma file should have the font styles. Don't need to follow to a T but maybe use it as a guidance on what should be consistent to each other. Ex. The "Learn more..", "Download BC Wallet", "Don't have a digital wallet", "This QR code..." should be the same font size. Thanks!

— Reply to this email directly, view it on GitHub https://github.com/bcgov/vc-authn-oidc/pull/572#issuecomment-2221288279, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIGY77EPWVZUNSCVUWVNSBTZLWFBLAVCNFSM6AAAAABKME36EOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRRGI4DQMRXHE . You are receiving this because you were mentioned.Message ID: @.***>

Gavinok commented 1 week ago

I have updated the font sizes to be consistent. 2024-07-11_16-04-26

Gavinok commented 3 days ago

@loneil Forgot to add that file. Thanks for testing

esune commented 21 hours ago

@Gavinok one last conflict to resolve and we can merge this.