-
Notifications
You must be signed in to change notification settings - Fork 22
Wallet sdk credential and proof plus display #350
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wallet sdk credential and proof plus display #350
Conversation
45a5915
to
50ba434
Compare
926e723
to
afa7abd
Compare
50ba434
to
f7bf413
Compare
Signed-off-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
72e9c00
to
7846041
Compare
Signed-off-by: Berend Sliedrecht <[email protected]>
54e7d87
to
c586b15
Compare
Signed-off-by: Berend Sliedrecht <[email protected]>
c586b15
to
838a29d
Compare
Signed-off-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
// TODO: remove this? | ||
credentialCategory?: CredentialCategoryMetadata['credentialCategory'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove it? I think we're using this to retrieve the PID record
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah mainly because we only really use it for the PID. Just wondering about how relevant that is in a general SDK.
@@ -0,0 +1,10 @@ | |||
import { useCredentialRecords } from './useCredentialRecords' | |||
|
|||
export const useCredentialRecordById = (id: string) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for all these hooks we need to update it to use the for display variant by default (which also includes the record i think?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the useCredentials
now returns display information, and the useCredentialRecords
only the record. useCredentials
will be a public hook and useCredentialRecords
will be an internal hook
Signed-off-by: Berend Sliedrecht <[email protected]>
Credential and proof display
I have migrated quite some functionality from
@packages/agent
to@paradym/wallet-sdk
. This mainly includes the display functionality for credentials and proofs. It also includes a lot of hooks, invitation functionality and utils. Theeasypid
app now uses some of this new functionality from the wallet sdk, but it has to be refactored to use theParadymWalletSdk
instance, so the usage is not too relevant. I mainly changed it so that migrating to usage of theParadymWalletSdk
is easier, as all the types already line up.I also used
Error
internally, as all cases that were reached were fatal. At a higher level,ParadymWalletSdk
, we can wrap this and convert into an object with asuccess
or something equivalent.Git also did not correctly remap some files, even when using
git mv
sadly.There are still some tasks related to this, like improving the DX of the proof display attributes, but I would like to do this in a new PR as +3.3k and -2.6k is a bit much.