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.

Sign up for GitHub

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

Jump to bottom

feat(extension): LW-9178 collateral output in tx activity details #844

Merged
merged 13 commits into from
Mar 25, 2024
Merged

feat(extension): LW-9178 collateral output in tx activity details #844

merged 13 commits into from
Mar 25, 2024

Conversation

renanvalentin
Copy link
Contributor

@renanvalentin renanvalentin commented Jan 18, 2024 •
edited
Loading

Checklist

  • JIRA - <link>
  • Proper tests implemented
  • Screenshots added.

Testing

Visit https://dark-worm-90.deno.dev/

Screenshots

Screenshot 2024-02-14 at 15 31 59

Screenshot 2024-02-14 at 15 39 16

@renanvalentin renanvalentin self-assigned this Jan 18, 2024
@github-actions github-actions bot added the browser Changes to the browser application. label Jan 18, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 18, 2024 •
edited
Loading

Allure report

allure-report-publisher generated test report!

smokeTests: ✅ test report for b038bdb4

passed failed skipped flaky total result
Total 30 0 0 0 30 ✅

@coveralls
Copy link

coveralls commented Jan 18, 2024 •
edited
Loading

Coverage Status

Changes unknown
when pulling b038bdb on lw-9178-cip40
into ** on main**.

@renanvalentin renanvalentin marked this pull request as ready for review January 31, 2024 13:07
@renanvalentin renanvalentin requested a review from a team as a code owner January 31, 2024 13:07
@renanvalentin renanvalentin changed the base branch from main to lw-9669-collateral-dialogue February 14, 2024 18:35
@renanvalentin renanvalentin changed the title feat(extension): [LW-9178] add utility function to calculate the tx collateral feat(extension): LW-9178 collateral output in tx activity details Feb 14, 2024
pczeglik-iohk
pczeglik-iohk reviewed Feb 19, 2024
View reviewed changes
Copy link
Contributor

@pczeglik-iohk pczeglik-iohk left a comment

Choose a reason for hiding this comment

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

Code looks good. I posted a few minor improvements. Let me know what do you think.

Thank you @renanvalentin

apps/browser-extension-wallet/src/stores/slices/activity-detail-slice.ts Outdated Show resolved Hide resolved
apps/browser-extension-wallet/src/stores/slices/activity-detail-slice.ts Outdated Show resolved Hide resolved
packages/core/src/ui/components/ActivityDetail/TransactionDetails.tsx Outdated Show resolved Hide resolved
packages/core/src/ui/components/ActivityDetail/Collateral.tsx Outdated Show resolved Hide resolved
packages/ui/src/design-system/tooltip/tooltip-root.css.ts Outdated Show resolved Hide resolved
Base automatically changed from lw-9669-collateral-dialogue to main February 19, 2024 12:21
pczeglik-iohk
pczeglik-iohk approved these changes Feb 20, 2024
View reviewed changes
Copy link
Contributor

@pczeglik-iohk pczeglik-iohk left a comment

Choose a reason for hiding this comment

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

Thank you @renanvalentin

@renanvalentin
Copy link
Contributor Author

@lucas-barros @pczeglik-iohk can you please review this commit: 1210a21

Screenshot 2024-03-12 at 13 51 18
Screenshot 2024-03-12 at 13 51 23

@lucas-barros
Copy link

@lucas-barros @pczeglik-iohk can you please review this commit: 1210a21

Screenshot 2024-03-12 at 13 51 18 Screenshot 2024-03-12 at 13 51 23

@renanvalentin looks good! My only observation is to check if hasPhase2ValidationFailed could be shared.

pczeglik
pczeglik reviewed Mar 18, 2024
View reviewed changes
packages/core/src/ui/components/ActivityDetail/Collateral.tsx
switch (status) {
case 'review':
case 'error':
return t('package.core.activityDetails.collateral.tooltip.info');
Copy link

Choose a reason for hiding this comment

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

Maybe it would be good to have a dedicated tooltip's text for review and error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a status bar below the collateral when phase 2 fails.

@github-actions github-actions bot removed browser Changes to the browser application. ui-toolkit labels Mar 25, 2024
@renanvalentin renanvalentin enabled auto-merge (squash) March 25, 2024 13:09
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Mar 25, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@renanvalentin renanvalentin merged commit 2db23bf into main Mar 25, 2024
13 of 16 checks passed
@renanvalentin renanvalentin deleted the lw-9178-cip40 branch March 25, 2024 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pczeglik pczeglik pczeglik left review comments

@pczeglik-iohk pczeglik-iohk pczeglik-iohk approved these changes

@lucas-barros lucas-barros lucas-barros approved these changes

@mkazlauskas mkazlauskas Awaiting requested review from mkazlauskas

@AngelCastilloB AngelCastilloB Awaiting requested review from AngelCastilloB

Assignees

@renanvalentin renanvalentin

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@renanvalentin @coveralls @lucas-barros @pczeglik @pczeglik-iohk