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

[FIX] Add asset warning details #1850

Open
wants to merge 5 commits into
base: dependabot/npm_and_yarn/major-a4fa2e6183
Choose a base branch
from
Open

[FIX] Add asset warning details #1850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c7f2d21
fixes alignment on add asset warning, labels warning by issuer
aristidesstaffieri Feb 12, 2025
f490344
Added translations
aristidesstaffieri Feb 12, 2025
fbcf4bf
updates snapshots for add asset token warning changes
aristidesstaffieri Feb 12, 2025
8f193b6
remove new snapshots for wrong targets
aristidesstaffieri Feb 12, 2025
99f9f43
updats chromium darwin snapshots for add asset tests
aristidesstaffieri Feb 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified ...s/addAsset.test.ts-snapshots/manage-assets-unverified-token-chromium-darwin.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ...sts/addAsset.test.ts-snapshots/manage-assets-verified-token-chromium-darwin.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
11 changes: 8 additions & 3 deletions extension/src/popup/components/WarningMessages/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -706,13 +706,15 @@ export const TokenWarning = ({
isVerifiedToken,
verifiedLists = [],
handleAddToken,
isCustomToken,
}: {
domain: string;
code: string;
onClose: () => void;
isVerifiedToken: boolean;
verifiedLists?: string[];
handleAddToken: null | (() => Promise<void>);
isCustomToken: boolean;
}) => {
const { t } = useTranslation();
const warningRef = useRef<HTMLDivElement>(null);
Expand Down Expand Up @@ -768,8 +770,11 @@ export const TokenWarning = ({
<div className="TokenWarning__description__icon">
<Icon.User02 />
</div>
<div className="TokenWarning__description__text">
{t("Add Asset Trustline")}
<div
className="TokenWarning__description__text"
data-testid="DescriptionLabel"
>
{isCustomToken ? t("Add Token") : t("Add Asset Trustline")}
</div>
</div>
</div>
Expand Down Expand Up @@ -845,7 +850,7 @@ export const TokenWarning = ({
isSubmitting || submitStatus === ActionStatus.PENDING
}
>
{t("Add asset")}
{isCustomToken ? t("Add token") : t("Add asset")}
</Button>
</div>{" "}
</div>
Expand Down
1 change: 0 additions & 1 deletion extension/src/popup/components/WarningMessages/styles.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@
position: fixed;
height: fit-content;
width: 100%;
bottom: -1.5rem;
left: 0;

&__wrapper {
Expand Down
64 changes: 64 additions & 0 deletions extension/src/popup/components/__tests__/WarningMessages.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import React from "react";
import { render, screen, waitFor } from "@testing-library/react";

import { TokenWarning } from "../WarningMessages";
import { Wrapper } from "popup/__testHelpers__";
import { ActionStatus } from "@shared/api/types";
import { ROUTES } from "popup/constants/routes";

describe("Token Warning", () => {
it("should correctly label warning as token", async () => {
render(
<Wrapper
routes={[ROUTES.addAsset]}
state={{
settings: {
submitStatus: ActionStatus.IDLE,
},
}}
>
<TokenWarning
domain="example.com"
code="E2E"
onClose={() => {}}
isVerifiedToken={true}
verifiedLists={[]}
handleAddToken={() => Promise.resolve()}
isCustomToken={true}
/>
</Wrapper>,
);
await waitFor(() => screen.getByTestId("TokenWarning"));
expect(screen.getByTestId("DescriptionLabel")).toHaveTextContent(
"Add Token",
);
expect(screen.getByTestId("add-asset")).toHaveTextContent("Add token");
});
it("should correctly label warning as asset", async () => {
render(
<Wrapper
routes={[ROUTES.addAsset]}
state={{
settings: {
submitStatus: ActionStatus.IDLE,
},
}}
>
<TokenWarning
domain="example.com"
code="E2E"
onClose={() => {}}
isVerifiedToken={true}
verifiedLists={[]}
handleAddToken={() => Promise.resolve()}
isCustomToken={false}
/>
</Wrapper>,
);
await waitFor(() => screen.getByTestId("TokenWarning"));
expect(screen.getByTestId("DescriptionLabel")).toHaveTextContent(
"Add Asset",
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we add the whole string here for clarity? E.g. "Add Asset Trustline"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup I see, adding that. Thanks for catching it!

);
expect(screen.getByTestId("add-asset")).toHaveTextContent("Add asset");
});
});
1 change: 1 addition & 0 deletions extension/src/popup/components/manageAssets/ManageAssetRows/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ export const ManageAssetRows = ({
)}
{showUnverifiedWarning && (
<TokenWarning
isCustomToken={isContractId(suspiciousAssetData.issuer)}
handleAddToken={handleAddToken}
domain={suspiciousAssetData.domain}
code={suspiciousAssetData.code}
Expand Down
2 changes: 2 additions & 0 deletions extension/src/popup/locales/en/translation.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
"Add manually": "Add manually",
"Add network": "Add network",
"Add new list": "Add new list",
"Add token": "Add token",
"Add Token": "Add Token",
"Add XLM": "Add XLM",
"Address": "Address",
"Addresses are uppercase and begin with letters “G“, “M“, or “C“": "Addresses are uppercase and begin with letters “G“, “M“, or “C“.",
Expand Down
2 changes: 2 additions & 0 deletions extension/src/popup/locales/pt/translation.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
"Add manually": "Add manually",
"Add network": "Add network",
"Add new list": "Add new list",
"Add token": "Add token",
"Add Token": "Add Token",
"Add XLM": "Add XLM",
"Address": "Address",
"Addresses are uppercase and begin with letters “G“, “M“, or “C“": "Addresses are uppercase and begin with letters “G“, “M“, or “C“.",
Expand Down
Loading