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: new proof request loading screen #1243

Merged
merged 35 commits into from
Oct 3, 2024

Conversation

jleach
Copy link
Contributor

@jleach jleach commented Aug 22, 2024

Summary of Changes

Improved loading experience and streamlined testing

  • New Loading Animations: You'll see a smoother loading experience when establishing connections and handling proof requests.
  • Centralized Mock Data: Test data for OOB, Connection, Offer, and Proof is now neatly organized in react-hooks.ts, making tests easier to manage.
  • Test & Snapshot Updates: Unit tests and snapshots have been refreshed to reflect these changes.

This summary focuses on the key user-facing improvement (loading animations) and the behind-the-scenes benefit of centralized mock data, while also acknowledging the necessary test updates. It uses positive language and avoids technical jargon that might not be familiar to all reviewers.

Related Issues

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.

  • All commits contain a DCO Signed-off-by line (we use the DCO GitHub app to enforce this);
  • Updated LICENSE-3RD-PARTY.md for any added dependencies or vendored components;
  • Updated documentation as needed for changed code and new or modified features;
  • Added sufficient tests so that overall code coverage is not reduced.

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 🤓

  • Read our contribution guide at least once; it will save you a few review cycles!
  • Your PR will likely not be reviewed until all the above boxes are checked and all automated tests have passed.

PR template adapted from the Python attrs project.

@jleach jleach force-pushed the feat/new-prof-skelly branch 5 times, most recently from 988e505 to a6c6cc4 Compare September 24, 2024 22:34
@jleach jleach marked this pull request as ready for review September 25, 2024 19:15
@jleach jleach force-pushed the feat/new-prof-skelly branch 2 times, most recently from 2d699ea to 2d5875e Compare October 1, 2024 19:53
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we not commit this file?

@@ -10,6 +10,7 @@ import {
import { useBasicMessages, useCredentialByState, useProofByState } from '@credo-ts/react-hooks'
import { ProofCustomMetadata, ProofMetadata } from '@hyperledger/aries-bifold-verifier'
import { useEffect, useState } from 'react'
// import { log } from 'console'
Copy link
Contributor

Choose a reason for hiding this comment

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

should probably remove

Comment on lines 44 to 58
marginLeft: 0,
backgroundColor: ColorPallet.grayscale.lightGrey,
width: 40,
borderTopLeftRadius: 15,
borderBottomLeftRadius: 15,
}}
/>
<View
style={{
marginLeft: -22,
marginTop: 20,
backgroundColor: ColorPallet.grayscale.lightGrey,
height: 45,
width: 45,
borderRadius: 5,
Copy link
Contributor

Choose a reason for hiding this comment

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

These width / margin numbers seem somewhat arbitrary, could they be explained or named?

Comment on lines 61 to 65
<View style={{ marginLeft: 15, marginTop: 15, marginBottom: 15 }}>
<View style={[myStyle.rectangle, { width: 240, height: 20 }]} />
<View style={[myStyle.rectangle, { width: 145, height: 25 }]} />
<View style={[myStyle.rectangle, { width: 75, height: 20, marginTop: 20 }]} />
<View style={[myStyle.rectangle, { width: 210, height: 25 }]} />
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

@bryce-mcmath bryce-mcmath self-requested a review October 1, 2024 20:58
Copy link
Contributor

@bryce-mcmath bryce-mcmath left a comment

Choose a reason for hiding this comment

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

Sorry, meant to request changes but accidentally approved

bryce-mcmath
bryce-mcmath previously approved these changes Oct 2, 2024
@bryce-mcmath
Copy link
Contributor

The xcode file is still included but that isn't a big deal it's just an EOF

@jleach
Copy link
Contributor Author

jleach commented Oct 2, 2024

The xcode file is still included but that isn't a big deal it's just an EOF

It was just a EOF but I fixed it anyway.

Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
Signed-off-by: Jason C. Leach <[email protected]>
@jleach jleach merged commit fd3f90d into openwallet-foundation:main Oct 3, 2024
5 checks passed
@jleach jleach deleted the feat/new-prof-skelly branch October 3, 2024 16:36
Copy link

sonarcloud bot commented Oct 3, 2024

al-rosenthal pushed a commit to al-rosenthal/bifold-wallet that referenced this pull request Oct 9, 2024
al-rosenthal pushed a commit to al-rosenthal/bifold-wallet that referenced this pull request Oct 10, 2024
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