Skip to content
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

feat: credential overview #4

Merged
merged 40 commits into from
Jun 2, 2023
Merged

feat: credential overview #4

merged 40 commits into from
Jun 2, 2023

Conversation

janrtvld
Copy link
Contributor

@janrtvld janrtvld commented May 29, 2023

RPReplay_Final1685371507.mov

Jan and others added 21 commits May 25, 2023 11:51
Signed-off-by: Jan <[email protected]>
Signed-off-by: Jan <[email protected]>
Signed-off-by: Jan <[email protected]>
Signed-off-by: Jan <[email protected]>
Signed-off-by: Jan <[email protected]>
Signed-off-by: Jan <[email protected]>
Signed-off-by: Jan <[email protected]>
Signed-off-by: Jan <[email protected]>
Signed-off-by: Jan <[email protected]>
@TimoGlastra
Copy link
Member

Really like the recently added vs all credentials. it seems like a good way to clear the clutter! Maybe it could even be a favourites, or frequently used list in the future

Copy link
Member

@TimoGlastra TimoGlastra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@@ -12,7 +12,7 @@ import * as React from 'react'

type W3cCredentialRecordState = {
w3cCredentialRecords: Array<W3cCredentialRecord>
loading: boolean
isLoading: boolean
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with this change. Reason I kept it loading as that's the pattern used by the react-hooks AFJ extension and if we start using more types from there it will be inconsistent.

packages/app/features/wallet/WalletScreen.tsx Show resolved Hide resolved
packages/app/utils/utils.ts Outdated Show resolved Hide resolved
Signed-off-by: Jan <[email protected]>
Base automatically changed from feat/credential-offer to main May 31, 2023 09:49
Jan and others added 5 commits June 1, 2023 16:31
Signed-off-by: Jan <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Co-authored-by: Timo Glastra <[email protected]>
Signed-off-by: Jan <[email protected]>
Jan added 6 commits June 1, 2023 16:32
@janrtvld janrtvld marked this pull request as ready for review June 1, 2023 14:39
@janrtvld
Copy link
Contributor Author

janrtvld commented Jun 1, 2023

I've removed the bottom tab bar and moved the scan button to the top. Not sure if we want the color? Or should we just make it a plain button?

IMG_46502136941B-1

@TimoGlastra
Copy link
Member

I've removed the bottom tab bar and moved the scan button to the top. Not sure if we want the color? Or should we just make it a plain button?

Maybe we can add some gradient to the button. But this is just a random thought so please ignore this if I'm saying something stupid

@TimoGlastra
Copy link
Member

TimoGlastra commented Jun 1, 2023

Also, seems there's merge confilcts

@janrtvld
Copy link
Contributor Author

janrtvld commented Jun 1, 2023

Whats the trick for the CI? :) @TimoGlastra

Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
@TimoGlastra
Copy link
Member

Pushed some changs to fix the CI @janrxyz.

This was the issue:

  ➤ YN0000: │ @@ -2624,9 +2624,8 @@
  ➤ YN0000: │      "@aries-framework/openid4vc-client": ^0.4.0-alpha.126
  ➤ YN0000: │      "@aries-framework/react-hooks": 0.4.2
  ➤ YN0000: │      "@aries-framework/react-native": ^0.4.0-alpha.126
  ➤ YN0000: │      "@hyperledger/aries-askar-react-native": 0.1.0-dev.8
  ➤ YN0028: │ -    "@sphereon/openid4vci-client": ^0.4.0
  ➤ YN0000: │    peerDependencies:
  ➤ YN0000: │      "@hyperledger/aries-askar-react-native": 0.1.0-dev.8
  ➤ YN0000: │    languageName: unknown
  ➤ YN0000: │    linkType: soft
  ➤ YN0000: │ 
  ➤ YN0028: │ The lockfile would have been modified by this install, which is explicitly forbidden.

apps/expo/app/(home)/wallet.tsx Outdated Show resolved Hide resolved
apps/expo/app/(home)/wallet.tsx Outdated Show resolved Hide resolved
packages/app/components/CredentialCard.tsx Outdated Show resolved Hide resolved
packages/app/features/wallet/WalletScreen.tsx Show resolved Hide resolved
packages/app/features/wallet/WalletScreen.tsx Outdated Show resolved Hide resolved
packages/app/features/wallet/WalletScreen.tsx Outdated Show resolved Hide resolved
packages/app/features/wallet/WalletScreen.tsx Outdated Show resolved Hide resolved
packages/ui/src/content/Icon.tsx Show resolved Hide resolved
packages/ui/src/tamagui.config.ts Outdated Show resolved Hide resolved
Comment on lines +67 to +101
<YStack g="lg" width="100%">
<Heading variant="h3" textAlign="left">
Recently added
</Heading>
<ZStack
f={0}
flexBasis="auto"
height={
BASE_CREDENTIAL_CARD_HEIGHT + records.slice(0, 3).length * CREDENTIAL_TOP_INFO_OFFSET
}
>
{records.slice(0, 3).map((x, idx) => {
const credential = x.credential
return (
<XStack
key={x.id}
mt={72 * idx}
br={borderRadiusSizes.xl}
borderColor="$lightTranslucent"
borderWidth={0.5}
>
<CredentialCard
onPress={() => navigateToCredentialDetail(x.id)}
iconUrl={credential.issuer.iconUrl}
name={credential.name}
issuerName={credential.issuer.name}
subtitle={credential.description}
bgColor={credential?.credentialBranding?.backgroundColor}
shadow={false}
/>
</XStack>
)
})}
</ZStack>
</YStack>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for now, but I think we should extract this to a spearate component in the future. That will make it cleaner IMO:

	<CredentialCardsList 
	  title="Recently Added"
	  credentials={record.slice(0 ,3)}
	  onPressCredential={(credential) => navigateToCredentialDetail(credential.id)}
	/>

And the implementation:

const CredentialCardsList = ({ credentials }) => {

   const height = BASE_CREDENTIAL_CARD_HEIGHT + credentials.length * CREDENTIAL_TOP_INFO_OFFSET

  return (
        <YStack g="lg" width="100%">
          <Heading variant="h3" textAlign="left">
            Recently added
          </Heading>
          <ZStack
            f={0}
            flexBasis="auto"
            height={height}
          >
            {credentials.map((c, idx) => {
              const credential = x.credential
              return (
                <XStack
                  key={x.id}
                  mt={72 * idx}
                  br={borderRadiusSizes.xl}
                  borderColor="$lightTranslucent"
                  borderWidth={0.5}
                >
                  <CredentialCard
                    onPress={() => navigateToCredentialDetail(x.id)}
                    iconUrl={credential.issuer.iconUrl}
                    name={credential.name}
                    issuerName={credential.issuer.name}
                    subtitle={credential.description}
                    bgColor={credential?.credentialBranding?.backgroundColor}
                    shadow={false}
                  />
                </XStack>
              )
            })}
          </ZStack>
        </YStack>
  )
}

return (
<XStack
key={x.id}
mt={72 * idx}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another magic number. Is the overlap 100PX and we render 72PX?

@TimoGlastra TimoGlastra merged commit 93e5ff1 into main Jun 2, 2023
@TimoGlastra TimoGlastra deleted the feat/credential-overview branch June 2, 2023 12:30
@janrtvld janrtvld restored the feat/credential-overview branch June 2, 2023 12:47
janrtvld pushed a commit that referenced this pull request Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants