feat: Created a way to toggle specific buttons on specific states #39
Conversation
Tommylans
commented
Dec 6, 2022
•
Tommylans
commented
Signed-off-by: Tom Lanser <[email protected]>
| const actions = [ | ||
| isAcceptable ? ( | ||
| <ActionIcon key="accept" color="green"> | ||
| <IconCheck size={16} stroke={1.5} onClick={onAccept} /> | ||
| </ActionIcon> | ||
| ) : null, | ||
|
|
||
| isDeclineable ? ( | ||
| <ActionIcon key="reject" color="red" onClick={onDecline}> | ||
| <IconX size={16} stroke={1.5} /> | ||
| </ActionIcon> | ||
| ) : null, | ||
|
|
||
| <ActionIcon key="delete" color="red" onClick={onDelete}> | ||
| <IconTrash size={16} stroke={1.5} /> | ||
| </ActionIcon>, | ||
| ] |
There was a problem hiding this comment.
| const actions = [ | |
| isAcceptable ? ( | |
| <ActionIcon key="accept" color="green"> | |
| <IconCheck size={16} stroke={1.5} onClick={onAccept} /> | |
| </ActionIcon> | |
| ) : null, | |
| isDeclineable ? ( | |
| <ActionIcon key="reject" color="red" onClick={onDecline}> | |
| <IconX size={16} stroke={1.5} /> | |
| </ActionIcon> | |
| ) : null, | |
| <ActionIcon key="delete" color="red" onClick={onDelete}> | |
| <IconTrash size={16} stroke={1.5} /> | |
| </ActionIcon>, | |
| ] | |
| const actions = [ | |
| <ActionIcon key="accept" color="green"> | |
| <IconCheck size={16} stroke={1.5} onClick={onAccept} /> | |
| </ActionIcon> | |
| <ActionIcon key="reject" color="red" onClick={onDecline}> | |
| <IconX size={16} stroke={1.5} /> | |
| </ActionIcon> | |
| <ActionIcon key="delete" color="red" onClick={onDelete}> | |
| <IconTrash size={16} stroke={1.5} /> | |
| </ActionIcon>, | |
| ].filter(Boolean) |
There was a problem hiding this comment.
With this change the buttons are always visible right? I see the filter but not really the use of the isLoading variable or did you left this out because the other comment to have a constant of the states?
There was a problem hiding this comment.
Ah no it was a mistake. However you can just check if it is acceptable when onAccept is supplied. That would mean one less prop and no mismatch in state. Filtering out null is not required then.
| {records.map((record) => { | ||
| const recordState = record.state | ||
|
|
||
| const isLoading = |
There was a problem hiding this comment.
It might be nice to abstract these loading states into an array in a constant file. like const connectionLoadingStates = [DidExchangeState.RequestSent,...]. Same goes for the credential and proof.
There was a problem hiding this comment.
With a function of like, isRecordWaitingForResponse(record)
There was a problem hiding this comment.
So the idea is to have a list in the RecordActions component with a list of states that are loading?
There was a problem hiding this comment.
No, just a list of the enums which have a state where we are waiting for a response.
The function would take a record and check if its state is in the list. Can work out an example tomorrow if I didn't explain it good enough.
| @@ -39,6 +39,7 @@ export const CredentialsScreen = () => { | |||
| connections={connectionRecords} | |||
| onDelete={(credential) => agent?.credentials.deleteById(credential.id)} | |||
| onAccept={(credential) => acceptCredential(credential)} | |||
| onDecline={(credential) => agent?.credentials.declineOffer(credential.id)} | |||
There was a problem hiding this comment.
Not too sure how, but it might be nice if we can always have the agent be defined when we are in the toolbox view (after initialization). Could you create an issue for that? We can resolve it in the future.
There was a problem hiding this comment.
I created the Issue #43
…using now. Signed-off-by: Tom Lanser <[email protected]>
|
@Tommylans there are some linting issues. Will review again. |
Signed-off-by: Tom Lanser <[email protected]>
| return { | ||
| isAcceptable: ConnectionUtil.acceptStates.includes(connection.state), | ||
| } |
There was a problem hiding this comment.
Why is this returned as an object? I would say a boolean is good enough here.
| @@ -0,0 +1,27 @@ | |||
| import type { ConnectionRecord } from '@aries-framework/core' | |||
There was a problem hiding this comment.
Could you rename it to ConnectionsUtil.ts? The modules are always plural so its a bit nice in sync.
| return { | ||
| isAcceptable: CredentialUtil.acceptStates.includes(credential.state), | ||
| isDeclineable: CredentialUtil.declineStates.includes(credential.state), |
There was a problem hiding this comment.
AH I see. I think, how verbose it might seen, two methods would be better. isCredentialWaitingForInputAndIsDeclineOrMaybeSomethingElseThisIsGettingVeryLong. :)
not too sure about that name though,
Signed-off-by: Tom Lanser <[email protected]>
