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: export json in activity detail #1331

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

shawnbusuttil
Copy link
Contributor

Checklist


Proposed solution

Add option to download tx file in acitvity detail.

Testing

Describe here, how the new implementation can be tested.
Provide link or briefly describe User Acceptance Criteria/Tests that need to be met

Screenshots

image

@shawnbusuttil shawnbusuttil added the browser Changes to the browser application. label Jul 29, 2024
@shawnbusuttil shawnbusuttil self-assigned this Jul 29, 2024
@shawnbusuttil shawnbusuttil requested a review from a team as a code owner July 29, 2024 16:30
@github-actions github-actions bot removed the browser Changes to the browser application. label Jul 29, 2024
@pczeglik-iohk
Copy link
Contributor

pczeglik-iohk commented Jul 29, 2024

Allure Report

allure-report-publisher generated test report!

smokeTests: ✅ test report for 5883050a

passed failed skipped flaky total result
Total 32 0 0 0 32

processReports: ✅ test report for a76f8fac

passed failed skipped flaky total result
Total 32 0 0 0 32

Copy link
Contributor

@greatertomi greatertomi left a comment

Choose a reason for hiding this comment

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

Looks good to me. Amazing work Shawn 🎉

Copy link
Contributor

@danielmain danielmain left a comment

Choose a reason for hiding this comment

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

Left an optional improvement. Not sure if it still works that way

@@ -95,6 +101,11 @@ export const SharedWalletTransactionDetailsWrapper = withAddressBookContext(
[addressToNameMap, amountTransformer, cardanoCoin, txSummary]
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't you also need coSigners to be watched for changes?

Suggested change
[addressToNameMap, amountTransformer, cardanoCoin, txSummary]
[addressToNameMap, amountTransformer, cardanoCoin, txSummary, coSigners]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved in slack.

@shawnbusuttil shawnbusuttil enabled auto-merge (squash) August 2, 2024 17:36
@shawnbusuttil shawnbusuttil disabled auto-merge August 23, 2024 12:06
Copy link

sonarcloud bot commented Aug 23, 2024

@rhyslbw rhyslbw marked this pull request as draft September 13, 2024 09:00
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.

5 participants