feat: unify UI for DIDComm and OpenID flows for Offer and details screens #1365
Conversation
Signed-off-by: Mostafa Gamal <[email protected]>
Signed-off-by: Mostafa Gamal <[email protected]>
Signed-off-by: Mostafa Gamal <[email protected]>
jleach
changed the title
jleach
changed the title
| import { credentialTextColor } from '../../utils/credential' | ||
| import { testIdWithKey } from '../../utils/testable' | ||
|
|
||
| type Props = { |
There was a problem hiding this comment.
Able to rename to CredentialDetailPrimaryHeaderProps to follow the app convetion?
| import { toImageSource } from '../../utils/credential' | ||
| import { testIdWithKey } from '../../utils/testable' | ||
|
|
||
| type Props = { |
There was a problem hiding this comment.
CredentialDetailSecondaryHeaderProps to follow convention.
|
|
||
| useEffect(() => { | ||
| const resolveOverlay = async () => { | ||
| if (!credentialDisplay || !bundleResolver || !i18n || !credentialDisplay.display) { |
There was a problem hiding this comment.
In the above two useEffect you put the guard clause if (x) return outside of the async function. Able to move this out or the other two in for consistency?
| ) | ||
| } | ||
|
|
||
| const screenEdges: Edge[] = ['left', 'right'] |
There was a problem hiding this comment.
This var seems orphaned down here. Can it be moved up with the others for readability?
|
|
||
| useEffect(() => { | ||
| const resolveOverlay = async () => { | ||
| if (!credentialDisplay || !bundleResolver || !i18n || !credentialDisplay.display) { |
| } | ||
|
|
||
| resolveOverlay() | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
For some reason, including 'credentialDisplay' in the dept array, causes infinite rendering loop. I spent enough time trying to dig deeper but couldn't figure out the reason. Removing this from the dept array makes rendering more stable.
| {credential && ( | ||
| <View style={{ marginHorizontal: 15, marginBottom: 16 }}> | ||
| <OpenIDCredentialCard credentialDisplay={display} /> | ||
| {/* <OpenIDCredentialCard credentialDisplay={display} /> */} |
Signed-off-by: Mostafa Gamal <[email protected]>
Signed-off-by: Mostafa Gamal <[email protected]>
|
|
Checked out and tested. LGTM. |
Summary of Changes
Unify UI for OpenID flow and Didcomm in regards to the following
Introducing OCA by adding mapping functions to map W3C credential metadata to default OCA branding overlay
Re-using Card components and creating some other re-usable components for both flows
Screenshots, videos, or gifs
UI Screens for OpenID Creds along with Didcomm, all look identical
DBC DIIP is W3C Cred, while the rest is Anoncreds
Breaking change guide
No breaking changes
Related Issues
Replace this text with issue #'s that are relevant to this PR. If there are none, simply enter N/A
Pull Request Checklist
Tick all boxes below to demonstrate that you have completed the respective task. If the item does not apply to your this PR check it anyway to make it apparent that there's nothing to do.
Signed-off-byline (we use the DCO GitHub app to enforce this)If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!
Pro Tip 🤓