cardano-foundation / cf-identity-wallet

Identity Wallet is an open source mobile application developed by the Cardano Foundation. It provides a digital solution for users to securely store, manage, and share their identifiers and verifiable credentials.
https://identity.cardanofoundation.org
Mozilla Public License 2.0
91 stars 12 forks source link

fix(ui): Validate btn on SSI agent screen disabled if connect fails #672

Closed Sotatek-DukeVu closed 2 months ago

Sotatek-DukeVu commented 2 months ago

Description

Enable validate button after connect fails

Checklist before requesting a review

Issue ticket number and link

Testing & Validation

Security

Code Review

Design Review

https://github.com/user-attachments/assets/d1956d09-2791-4b67-8b7e-56b265ee5acc

github-actions[bot] commented 2 months ago

Vercel PR (merge commit) deploy URL: https://cf-identity-wallet-4ozg4gzg2.vercel.app

iFergal commented 2 months ago

thanks, based on the video @Sotatek-DukeVu - I wonder if we can keep the validate button disabled until it's finished "trying" to connect? So once we get the red lines, it becomes enabled again. That would feel the most correct. What do you think @sdisalvo-crd ?

sdisalvo-crd commented 2 months ago

thanks, based on the video @Sotatek-DukeVu - I wonder if we can keep the validate button disabled until it's finished "trying" to connect? So once we get the red lines, it becomes enabled again. That would feel the most correct. What do you think @sdisalvo-crd ?

Yes, thanks @iFergal , that sounds correct. Also I wonder if we should disable the button while the red error message is displayed and re-enable it again removing the red error message once the content is edited.

iFergal commented 2 months ago

@sdisalvo-crd The whole point of the story was not to do that lol. As I mentioned, you may want to re-try without changing the values in case there was some other kind of error that happened.

Sotatek-DukeVu commented 2 months ago

@iFergal Ok. I will update it at tomorrow

sdisalvo-crd commented 2 months ago

Ok so we're talking about two different things here @iFergal, one is about entering a wrong value, the other is about capturing any other error that may have occurred and giving the possibility to the user to re-try that.

iFergal commented 2 months ago

@sdisalvo-crd I'm not necessarily talking about these crazy edge cases of network dying etc, like the generic errors.

I was just suggesting that if the boot or connect fails and we display the error, that we can just click the button again to try again. It just feels more natural to me - and it was already inconsistent before (before: if boot failed, button wasn't disabled but if connect failed, button was disabled)

On top of that, they will probably enter the URL by QR code - so it will be correct. So in case they scan again, now the text won't change, and they can't try again without restarting the app. There might've just been a weird error on KERIA that fixes itself once trying again - it's just safer to let them try again imo

Sotatek-DukeVu commented 2 months ago

@iFergal @sdisalvo-crd Done. Please help me check.

github-actions[bot] commented 2 months ago

Vercel PR (merge commit) deploy URL: https://cf-identity-wallet-f1arw69eu.vercel.app