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(ui): Flag addresses as own or foreign [LW-10061] #998

Merged
merged 3 commits into from
Apr 19, 2024

Conversation

DominikGuzei
Copy link
Member

@DominikGuzei DominikGuzei commented Mar 27, 2024

Checklist


Proposed solution

adds a new AddressTag component to the @lace/ui toolkit with variants:

  • tagging own addresses of the active wallet
  • tagging address book contacts (by address) with the contact name
  • tagging all other addresses as foreign

Integrates the logic + address tag component in DApp tx summary (from/to section) and activity feed details

Screenshots

DApp tx summary:
image

Tx Activity details:
Bildschirmfoto 2024-04-03 um 14 35 41

@DominikGuzei DominikGuzei self-assigned this Mar 27, 2024
@DominikGuzei DominikGuzei changed the title feat(ui): add address tag ui component LW-10061 feat(ui): add address tag ui component [LW-10061] Mar 27, 2024
@DominikGuzei DominikGuzei changed the title feat(ui): add address tag ui component [LW-10061] feat(ui): Flag addresses as own or foreign [LW-10061] Mar 27, 2024
Copy link

github-actions bot commented Mar 27, 2024

Allure report

allure-report-publisher generated test report!

smokeTests: ✅ test report for b1f7f455

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

@coveralls
Copy link

coveralls commented Mar 27, 2024

Coverage Status

coverage: 50.845% (+0.2%) from 50.62%
when pulling ceea841776508908b7447fbf62ef9cacc088c45f on feat/lw-10061-flag-addresses-as-own-or-foreign
into 985486a on main.

@DominikGuzei DominikGuzei force-pushed the feat/lw-10061-flag-addresses-as-own-or-foreign branch 3 times, most recently from 968b1a1 to 85ec6b4 Compare April 3, 2024 15:21
@DominikGuzei DominikGuzei added browser Changes to the browser application. ui-toolkit labels Apr 3, 2024
@DominikGuzei DominikGuzei marked this pull request as ready for review April 3, 2024 15:46
@DominikGuzei DominikGuzei requested a review from a team as a code owner April 3, 2024 15:46
color: vars.colors.$address_tag_own_color,
backgroundColor: vars.colors.$address_tag_own_bgColor,
},
[AddressTagVariants.Handle]: {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not used yet but was visible in the designs and so i already had implemented it before clarifying the smaller scope of the ticket. I would suggest to leave it here for later usage when we implement tagging handles

'addr_test1qzqgfww9svrzelxrnlml0nmdq4yevwke7ck7ae27u5ptmq5dwuq25p4hr0yxhg4pce0d6t7v4c0msy3vr3xppygn9ktqe77950'
);

const PXLAssetId = Wallet.Cardano.AssetId('1ec85dcee27f2d90ec1f9a1e4ce74a667dc9be8b184463223f9c960150584c');
Copy link
Member Author

@DominikGuzei DominikGuzei Apr 3, 2024

Choose a reason for hiding this comment

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

I just picked one existing asset I found in the SDK for this storybook example, could be any

module.exports = () => {
return {
testFramework: {
configFile: 'test/jest.config.js'
Copy link
Member Author

@DominikGuzei DominikGuzei Apr 3, 2024

Choose a reason for hiding this comment

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

This file is necessary to run Wallaby.js test runner in the core package which cannot handle our unusual jest setup (config nested in test folder`) by default

@DominikGuzei DominikGuzei force-pushed the feat/lw-10061-flag-addresses-as-own-or-foreign branch from 85ec6b4 to ceea841 Compare April 3, 2024 16:36
@github-actions github-actions bot removed browser Changes to the browser application. ui-toolkit labels Apr 3, 2024
@@ -0,0 +1,27 @@
import * as React from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this pattern.

Copy link
Contributor

@VanessaPC VanessaPC left a comment

Choose a reason for hiding this comment

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

LGTM

@DominikGuzei DominikGuzei force-pushed the feat/lw-10061-flag-addresses-as-own-or-foreign branch 7 times, most recently from 07aac21 to 3e39c9e Compare April 15, 2024 11:00
@DominikGuzei DominikGuzei force-pushed the feat/lw-10061-flag-addresses-as-own-or-foreign branch from 3e39c9e to b1f7f45 Compare April 19, 2024 15:36
Copy link

sonarcloud bot commented Apr 19, 2024

@DominikGuzei DominikGuzei merged commit 6bf8d47 into main Apr 19, 2024
13 of 15 checks passed
@DominikGuzei DominikGuzei deleted the feat/lw-10061-flag-addresses-as-own-or-foreign branch April 19, 2024 16:36
wklos-iohk pushed a commit that referenced this pull request Apr 22, 2024
* feat(ui): add address tag ui component LW-10061
* feat(extension): flag own, address book and foreign addresses in dapp txs LW-10061
* feat(extension): flag own, address book and foreign addresses in activity details LW-10061
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.

4 participants