feat(wallet-dashboard): style selected visual Assets. #4085

panteleymonchuk · 2024-11-15T18:20:35Z

Description of change

Please write a summary of your changes and why you made them.

Links to any relevant issues

Closes #3702
Closes #3703

Depends on #4159 because in the file useNftDetails.ts:18 we have flag isTransferable (not critical)

Be sure to reference any related issues by adding fixes #(issue).

Type of change

Choose a type of change, and delete any options that are not relevant.

Bug fix (a non-breaking change which fixes an issue)
Enhancement (a non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation Fix

How the change has been tested

Describe the tests that you ran to verify your changes.

Make sure to provide instructions for the maintainer as well as any relevant configurations.

Change checklist

Tick the boxes that are relevant to your changes, and delete any items that are not.

I have followed the contribution guidelines for this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
I have checked that new and existing unit tests pass locally with my changes

github-actions · 2024-11-19T15:37:56Z

This pull request has been deployed to Vercel.

Latest commit: 3e6609a

✅ Preview: https://apps-ui-k3bwiq3s3-iota1.vercel.app

github-actions · 2024-11-19T15:38:27Z

This pull request has been deployed to Vercel.

Latest commit: 3e6609a

✅ Preview: https://apps-backend-29a719c9ccbdb9b80c97b599a599d8953354da-eh61zw26s.vercel.app

apps/wallet-dashboard/components/Dialogs/Assets/views/DetailsView.tsx

github-actions · 2024-11-21T17:07:19Z

This pull request has been deployed to Vercel.

Latest commit: 093c3f4

✅ Preview: https://apps-ui-gmb4m6m9l-iota1.vercel.app

github-actions · 2024-11-21T17:07:39Z

This pull request has been deployed to Vercel.

Latest commit: 093c3f4

✅ Preview: https://apps-backend-29a719c9ccbdb9b80c97b599a599d8953354da-9zytokc9d.vercel.app

github-actions · 2024-11-21T17:40:56Z

This pull request has been deployed to Vercel.

Latest commit: b3291fb

✅ Preview: https://apps-ui-hqd7rines-iota1.vercel.app

github-actions · 2024-11-21T17:41:27Z

This pull request has been deployed to Vercel.

Latest commit: b3291fb

✅ Preview: https://apps-backend-29a719c9ccbdb9b80c97b599a599d8953354da-eqvcvqz9h.vercel.app

github-actions · 2024-11-21T17:41:49Z

This pull request has been deployed to Vercel.

Latest commit: b3291fb

✅ Preview: https://iota-rebased-explorer-092603f1de8088243ccb8b861b23e-em3q1ti7k.vercel.app

github-actions · 2024-11-21T17:42:07Z

This pull request has been deployed to Vercel.

Latest commit: b3291fb

✅ Preview: https://wallet-dashboard-a225fcfa1e2f852205a61a7cd1d5d93e92-715v9b7bi.vercel.app

marc2332 · 2024-11-22T08:29:26Z

apps/wallet-dashboard/components/Collapsible/Collapsible.tsx

+export function Collapsible({
+    title,
+    children,
+    defaultOpen,


Suggested change

defaultOpen,

defaultOpen = false,

apps/wallet-dashboard/components/Collapsible/Collapsible.tsx

apps/wallet-dashboard/components/ExplorerLink/ExplorerLink.tsx

apps/wallet-dashboard/components/tiles/AssetTileLink.tsx

apps/wallet-dashboard/components/Dialogs/Assets/views/DetailsView.tsx

github-actions · 2024-11-22T15:20:54Z

This pull request has been deployed to Vercel.

Latest commit: b15c9e8

✅ Preview: https://apps-ui-r3abi1al2-iota1.vercel.app

github-actions · 2024-11-22T15:21:15Z

This pull request has been deployed to Vercel.

Latest commit: b15c9e8

✅ Preview: https://apps-backend-29a719c9ccbdb9b80c97b599a599d8953354da-omixq143z.vercel.app

github-actions · 2024-11-22T15:21:23Z

This pull request has been deployed to Vercel.

Latest commit: b15c9e8

✅ Preview: https://iota-rebased-explorer-092603f1de8088243ccb8b861b23e-aw4xjm5cp.vercel.app

marc2332 · 2024-11-25T15:39:01Z

apps/core/src/hooks/useNftDetails.ts

+    const metaFields =
+        (nftFields as NftFields)?.metadata?.fields?.attributes?.fields ||
+        Object.entries(nftFields ?? {})
+            .filter(([key]) => key !== 'id')
+            .reduce(
+                (acc, [key, value]) => {
+                    acc.keys.push(key);
+                    acc.values.push(value as string);
+                    return acc;
+                },
+                { keys: [] as string[], values: [] as string[] },
+            );
+    const metaKeys: string[] = metaFields ? metaFields.keys : [];
+    const metaValues = metaFields ? metaFields.values : [];


a simple simplification

Suggested change

const metaFields =

(nftFields as NftFields)?.metadata?.fields?.attributes?.fields ||

Object.entries(nftFields ?? {})

.filter(([key]) => key !== 'id')

.reduce(

(acc, [key, value]) => {

acc.keys.push(key);

acc.values.push(value as string);

return acc;

},

{ keys: [] as string[], values: [] as string[] },

);

const metaKeys: string[] = metaFields ? metaFields.keys : [];

const metaValues = metaFields ? metaFields.values : [];

const { keys: metaKeys, values: metaValues } =

(nftFields as NftFields)?.metadata?.fields?.attributes?.fields ||

Object.entries(nftFields ?? {})

.filter(([key]) => key !== 'id')

.reduce(

(acc, [key, value]) => {

acc.keys.push(key);

acc.values.push(value as string);

return acc;

},

{ keys: [] as string[], values: [] as string[] },

);

marc2332 · 2024-11-25T15:39:08Z

apps/core/src/hooks/useNftDetails.ts

+        metaKeys,
+        metaValues,
+        formatMetaValue,
+


Suggested change

marc2332 · 2024-11-25T15:39:13Z

apps/core/src/hooks/useNftDetails.ts

+
+        isContainedInKiosk,
+        kioskItem,
+


Suggested change

marc2332 · 2024-11-25T15:42:43Z

apps/wallet-dashboard/components/Collapsible/Collapsible.tsx

+    title: string;
+    hideBorder?: boolean;
+    defaultExpanded?: boolean;
+    headerProps?: AccordionHeaderProps;
+    titleSize?: TitleSize;


Of all this props, only title and defaulyExpanded are ever used, I suppose you copied this component from the wallet or someting (not wrong), but does it mean its worth keeping the same props? Maybe it is, just saying

I moved original component to core, use it in the wallet-dashboard. Current duplicated component was removed.

github-actions · 2024-11-28T10:57:43Z

This pull request has been deployed to Vercel.

Latest commit: fc404d8

✅ Preview: https://iota-rebased-explorer-092603f1de8088243ccb8b861b23e-ojxkwrinj.vercel.app

github-actions · 2024-11-28T10:57:59Z

This pull request has been deployed to Vercel.

Latest commit: fc404d8

✅ Preview: https://wallet-dashboard-a225fcfa1e2f852205a61a7cd1d5d93e92-go1zs5kdg.vercel.app

github-actions · 2024-11-28T12:38:47Z

This pull request has been deployed to Vercel.

Latest commit: 7bd0c26

✅ Preview: https://apps-ui-mnqd0af3o-iota1.vercel.app

github-actions · 2024-11-28T12:39:18Z

This pull request has been deployed to Vercel.

Latest commit: 7bd0c26

✅ Preview: https://wallet-dashboard-a225fcfa1e2f852205a61a7cd1d5d93e92-bt400lni2.vercel.app

github-actions · 2024-11-28T12:39:45Z

This pull request has been deployed to Vercel.

Latest commit: 7bd0c26

✅ Preview: https://iota-rebased-explorer-092603f1de8088243ccb8b861b23e-ikghtim73.vercel.app

github-actions · 2024-11-28T12:39:53Z

This pull request has been deployed to Vercel.

Latest commit: 7bd0c26

✅ Preview: https://apps-backend-29a719c9ccbdb9b80c97b599a599d8953354da-qhiulsgg1.vercel.app

…and clarity

github-actions · 2024-11-28T12:54:38Z

This pull request has been deployed to Vercel.

Latest commit: 61a13f8

✅ Preview: https://apps-ui-ensfe39ma-iota1.vercel.app

github-actions · 2024-11-28T12:54:55Z

This pull request has been deployed to Vercel.

Latest commit: 61a13f8

✅ Preview: https://apps-backend-29a719c9ccbdb9b80c97b599a599d8953354da-559rvirkk.vercel.app

github-actions · 2024-11-28T12:55:04Z

This pull request has been deployed to Vercel.

Latest commit: 61a13f8

✅ Preview: https://iota-rebased-explorer-092603f1de8088243ccb8b861b23e-63kt470my.vercel.app

github-actions · 2024-11-28T12:56:00Z

This pull request has been deployed to Vercel.

Latest commit: 61a13f8

✅ Preview: https://wallet-dashboard-a225fcfa1e2f852205a61a7cd1d5d93e92-js7keu7r9.vercel.app

VmMad

Code looks good overall, just some suggestions.

The dialog should close after the NFT is sent, which currently doesn't happen

VmMad · 2024-11-29T10:27:01Z

apps/core/src/hooks/useNftDetails.ts

+                    acc.values.push(value as string);
+                    return acc;
+                },
+                { keys: [] as string[], values: [] as string[] },


I think Line 8 and this one could share a type

VmMad · 2024-11-29T10:29:52Z

apps/wallet-dashboard/components/Dialogs/Assets/AssetsDialog.tsx

+    setView: (view: AssetsDialogView | undefined) => void;
+}
+
+export interface FormValues {


If this interface needs too be exported I would say to rename it to something more specific

VmMad · 2024-11-29T10:36:00Z

apps/wallet-dashboard/app/(protected)/assets/page.tsx

@@ -25,6 +26,8 @@ const ASSET_CATEGORIES: { label: string; value: AssetCategory }[] = [
 ];

 export default function AssetsDashboardPage(): React.JSX.Element {
+    const [view, setView] = useState<AssetsDialogView | undefined>(AssetsDialogView.Details);


I think the view here is not necessary, it could be moved inside the AssetsDialog

VmMad · 2024-11-29T10:37:54Z

apps/wallet-dashboard/app/(protected)/assets/page.tsx

+                {view && (
+                    <AssetsDialog
+                        view={view}
+                        setView={setView}
+                        isOpen={!!selectedAsset}
+                        onClose={onCloseDialog}
+                        asset={selectedAsset}
+                    />
+                )}


Suggested change

{view && (

<AssetsDialog

view={view}

setView={setView}

isOpen={!!selectedAsset}

onClose={onCloseDialog}

asset={selectedAsset}

/>

)}

{selectedAsset && (

<AssetsDialog

onClose={() => setSelectedAsset(null)}

asset={selectedAsset}

/>

)}

VmMad · 2024-11-29T10:38:24Z

apps/wallet-dashboard/components/Dialogs/Assets/AssetsDialog.tsx

+interface AssetsDialogProps {
+    isOpen: boolean;
+    onClose: () => void;
+    asset: IotaObjectData | null;
+    view: AssetsDialogView;
+    setView: (view: AssetsDialogView | undefined) => void;
+}


Suggested change

interface AssetsDialogProps {

isOpen: boolean;

onClose: () => void;

asset: IotaObjectData | null;

view: AssetsDialogView;

setView: (view: AssetsDialogView | undefined) => void;

}

interface AssetsDialogProps {

onClose: () => void;

asset: IotaObjectData;

}

VmMad · 2024-11-29T10:50:56Z

apps/wallet-dashboard/components/Dialogs/Assets/AssetsDialog.tsx

+    async function onSubmit(values: FormValues) {
+        try {
+            await sendAsset.mutateAsync(values.to);
+        } catch (error) {
+            addNotification('Transfer transaction failed', NotificationType.Error);
+        }
+    }


After sending, first the dialog should be closed and then shown a notification, no?

VmMad · 2024-11-29T10:52:27Z

apps/wallet-dashboard/components/Dialogs/Assets/AssetsDialog.tsx

+    function onSendClose() {
+        setView(undefined);
+    }
+
+    function onSendBack() {
+        setView(AssetsDialogView.Details);
+    }


I feel it's a bit weird that the functions are defined by their view, rather than what they do, like, the function onSendClose should be called handleCloseDialog for example, and onSendBack should be called setDetailsView

VmMad · 2024-11-29T10:53:33Z

apps/wallet-dashboard/components/Dialogs/Assets/AssetsDialog.tsx

+
+    function onSendAssetSuccess() {
+        addNotification('Transfer transaction successful', NotificationType.Success);
+        router.push(ASSETS_ROUTE.path + '/assets');


this is not necessary, it's the same view, isn't it?

VmMad · 2024-11-29T10:56:34Z

apps/wallet-dashboard/components/Dialogs/Assets/AssetsDialog.tsx

The name should be singular, since it refers to a specific Asset, not multiple. Maybe even VisualAssetDialog or simply AssetDialog

VmMad · 2024-11-29T11:04:08Z

apps/wallet-dashboard/components/Dialogs/Assets/views/SendView.tsx

+                    </div>
+                    <div className="flex w-full flex-col gap-md">
+                        <div className="flex flex-col items-center gap-xxxs">
+                            <span className="text-title-lg text-neutral-10">{nftName}</span>


Let's try to avoid setting texts by hand, since we can forget adding darkmode colors

Suggested change

<span className="text-title-lg text-neutral-10">{nftName}</span>

<Title title={nftName} />

…style-selected-asset

github-actions · 2024-11-29T12:05:06Z

This pull request has been deployed to Vercel.

Latest commit: 16110bb

✅ Preview: https://apps-ui-ezy45gk6k-iota1.vercel.app

github-actions · 2024-11-29T12:05:22Z

This pull request has been deployed to Vercel.

Latest commit: 16110bb

✅ Preview: https://iota-rebased-explorer-092603f1de8088243ccb8b861b23e-f7j2lvxbz.vercel.app

github-actions · 2024-11-29T12:06:03Z

This pull request has been deployed to Vercel.

Latest commit: 16110bb

✅ Preview: https://apps-backend-29a719c9ccbdb9b80c97b599a599d8953354da-463o726ju.vercel.app

github-actions · 2024-11-29T12:06:47Z

This pull request has been deployed to Vercel.

Latest commit: 16110bb

✅ Preview: https://wallet-dashboard-a225fcfa1e2f852205a61a7cd1d5d93e92-c37yr7he9.vercel.app

brancoder

conflicts 🙏

…style-selected-asset # Conflicts: # apps/wallet-dashboard/providers/AppProviders.tsx

github-actions · 2024-12-02T12:59:15Z

This pull request has been deployed to Vercel.

Latest commit: 0bd5c50

✅ Preview: https://apps-ui-oouvt07te-iota1.vercel.app

github-actions · 2024-12-02T12:59:56Z

This pull request has been deployed to Vercel.

Latest commit: 0bd5c50

✅ Preview: https://iota-rebased-explorer-092603f1de8088243ccb8b861b23e-pm1c63vza.vercel.app

github-actions · 2024-12-02T13:00:43Z

This pull request has been deployed to Vercel.

Latest commit: 0bd5c50

✅ Preview: https://apps-backend-29a719c9ccbdb9b80c97b599a599d8953354da-2g78eskyu.vercel.app

github-actions · 2024-12-02T13:01:10Z

This pull request has been deployed to Vercel.

Latest commit: 0bd5c50

✅ Preview: https://wallet-dashboard-a225fcfa1e2f852205a61a7cd1d5d93e92-21bgl7ayl.vercel.app

brancoder

No notification or message is shown if an error occurs while sending the asset eventhough the error is caught in onSendAssetError callback. The Asset dialog is not dismissed and notification is not seen. The Asset dialog just remains open and Send button becomes enabled>

panteleymonchuk added the tooling Issues related to the Tooling team label Nov 15, 2024

panteleymonchuk changed the title ~~feat(wallet-dashboard): show details for Assets.~~ feat(wallet-dashboard): style selected visual Assets. Nov 15, 2024

panteleymonchuk linked an issue Nov 15, 2024 that may be closed by this pull request

Style selected visual asset #3702

Open

begonaalvarezd added the wallet-dashboard Issues related to the Wallet Dashboard dApp label Nov 18, 2024

panteleymonchuk force-pushed the tooling-dashboard/style-selected-asset branch from 029d55d to 3e6609a Compare November 19, 2024 15:35

panteleymonchuk marked this pull request as ready for review November 19, 2024 15:35

panteleymonchuk requested review from a team as code owners November 19, 2024 15:35

evavirseda requested changes Nov 21, 2024

View reviewed changes

apps/wallet-dashboard/components/Dialogs/Assets/views/DetailsView.tsx Outdated Show resolved Hide resolved

apps/wallet-dashboard/components/Dialogs/Assets/views/DetailsView.tsx Outdated Show resolved Hide resolved

panteleymonchuk force-pushed the tooling-dashboard/style-selected-asset branch from 3e6609a to 093c3f4 Compare November 21, 2024 17:05

panteleymonchuk requested review from evavirseda and VmMad November 22, 2024 08:31

marc2332 requested changes Nov 22, 2024

View reviewed changes

marc2332 self-requested a review November 25, 2024 15:23

marc2332 requested changes Nov 25, 2024

View reviewed changes

Base automatically changed from tooling-dashboard/style-selected-stake to develop November 25, 2024 16:20

panteleymonchuk added 3 commits November 25, 2024 19:36

feat(wallet-dashboard): style selected visual Assets.

4074f2b

refactor(core): destructure metaKeys and metaValues from attributes

9bb5dd0

refactor(wallet): move Collapsible component to core.

1f2eb82

Merge branch 'develop' into tooling-dashboard/style-selected-asset

7bd0c26

refactor(wallet-dashboard): rename handler functions for consistency …

61a13f8

…and clarity

VmMad requested changes Nov 29, 2024

View reviewed changes

panteleymonchuk and others added 4 commits November 29, 2024 13:08

Merge remote-tracking branch 'origin/develop' into tooling-dashboard/…

ed3e381

…style-selected-asset

fix(dashboard): PR conflicts

336468e

refactor(dashboard): update state for asset view, improve code

4ce390b

Merge branch 'develop' into tooling-dashboard/style-selected-asset

16110bb

panteleymonchuk requested a review from VmMad November 29, 2024 14:04

brancoder requested changes Dec 2, 2024

View reviewed changes

Merge remote-tracking branch 'origin/develop' into tooling-dashboard/…

0bd5c50

…style-selected-asset # Conflicts: # apps/wallet-dashboard/providers/AppProviders.tsx

panteleymonchuk requested a review from brancoder December 2, 2024 12:57

brancoder requested changes Dec 2, 2024

View reviewed changes

	<span className="text-title-lg text-neutral-10">{nftName}</span>
	<Title title={nftName} />

feat(wallet-dashboard): style selected visual Assets. #4085

Are you sure you want to change the base?

feat(wallet-dashboard): style selected visual Assets. #4085

Conversation

panteleymonchuk commented Nov 15, 2024 • edited Loading

Description of change

Links to any relevant issues

Type of change

How the change has been tested

Change checklist

github-actions bot commented Nov 19, 2024

github-actions bot commented Nov 19, 2024

github-actions bot commented Nov 21, 2024

github-actions bot commented Nov 21, 2024

github-actions bot commented Nov 21, 2024

github-actions bot commented Nov 21, 2024

github-actions bot commented Nov 21, 2024

github-actions bot commented Nov 21, 2024

Choose a reason for hiding this comment

github-actions bot commented Nov 22, 2024

github-actions bot commented Nov 22, 2024

github-actions bot commented Nov 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 28, 2024

github-actions bot commented Nov 28, 2024

github-actions bot commented Nov 28, 2024

github-actions bot commented Nov 28, 2024

github-actions bot commented Nov 28, 2024

github-actions bot commented Nov 28, 2024

github-actions bot commented Nov 28, 2024

github-actions bot commented Nov 28, 2024

github-actions bot commented Nov 28, 2024

github-actions bot commented Nov 28, 2024

VmMad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 29, 2024

github-actions bot commented Nov 29, 2024

github-actions bot commented Nov 29, 2024

github-actions bot commented Nov 29, 2024

brancoder left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 2, 2024

github-actions bot commented Dec 2, 2024

github-actions bot commented Dec 2, 2024

github-actions bot commented Dec 2, 2024

brancoder left a comment

Choose a reason for hiding this comment

panteleymonchuk commented Nov 15, 2024 •

edited

Loading