Skip to content

Commit

Permalink
Token table ordering (#5189)
Browse files Browse the repository at this point in the history
# Motivation

Currently, tokens table has a fixed list of tokens considered important
on top ordered (ICP, ckBTC, ckETH, ckUSDC) followed by SNS tokens
ordered by creation date. This ordering seems random for new people
entering the ecosystem, and only takes into account user balances after
a user takes action.

## Suggested order:
1. ICP is always on top regardless of user balance.
2. Tokens where a user has a higher than 0 balance:
a. First important token list in predefined order - ckBTC, ckETH, ckUSDC
  b. Sns tokens ordered alphabetically
3. Tokens where a user has 0 balance (same sub-groups as for tokens with
balance):
a. First important token list in predefined order - ckBTC, ckETH, ckUSDC
  b. Sns tokens ordered alphabetically

# Changes

- New `compareTokensForTokensTable` merged comparator.
- Apply sorting to sign-in and sign-out token table.

# Tests

- Old tests were updated to the new order.
- Tests for new comparators.
- Test that the Token page display tokens in the expected order (ckUSDC
token was added to the mock data).
- Updated the e2e test because the order changed after getting SNS
tokens.

# Todos

- [x] Add entry to changelog (if necessary).
  • Loading branch information
mstrasinskis authored Jul 16, 2024
1 parent c639711 commit 0994d30
Show file tree
Hide file tree
Showing 6 changed files with 310 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG-Nns-Dapp-unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ proposal is successful, the changes it released will be moved from this file to
#### Changed

* Reduce the frequency of checking if SNS neurons need to be refreshed.
* New token table order.

#### Deprecated

Expand Down
47 changes: 47 additions & 0 deletions frontend/src/lib/utils/tokens-table.utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { OWN_CANISTER_ID_TEXT } from "$lib/constants/canister-ids.constants";
import { CKBTC_UNIVERSE_CANISTER_ID } from "$lib/constants/ckbtc-canister-ids.constants";
import { CKETH_UNIVERSE_CANISTER_ID } from "$lib/constants/cketh-canister-ids.constants";
import { CKUSDC_UNIVERSE_CANISTER_ID } from "$lib/constants/ckusdc-canister-ids.constants";
import type { UserToken } from "$lib/types/tokens-page";
import {
createAscendingComparator,
createDescendingComparator,
mergeComparators,
} from "$lib/utils/responsive-table.utils";
import { TokenAmountV2 } from "@dfinity/utils";

const getTokenBalanceOrZero = (token: UserToken) =>
token.balance instanceof TokenAmountV2 ? token.balance.toUlps() : 0n;

export const compareTokensIcpFirst = createDescendingComparator(
(token: UserToken) => token.universeId.toText() === OWN_CANISTER_ID_TEXT
);

export const compareTokensWithBalanceFirst = createDescendingComparator(
(token: UserToken) => getTokenBalanceOrZero(token) > 0n
);

// These tokens should be placed before others (but after ICP)
// because they have significance within the Internet Computer ecosystem and deserve to be highlighted.
// Where the fixed order maps to a descending order in the market cap of the underlying native tokens.
const ImportantCkTokenIds = [
CKBTC_UNIVERSE_CANISTER_ID.toText(),
CKETH_UNIVERSE_CANISTER_ID.toText(),
CKUSDC_UNIVERSE_CANISTER_ID.toText(),
]
// To place other tokens (which get an index of -1) at the bottom.
.reverse();
export const compareTokensByImportance = createDescendingComparator(
(token: UserToken) => ImportantCkTokenIds.indexOf(token.universeId.toText())
);

export const compareTokensAlphabetically = createAscendingComparator(
({ title }: UserToken) => title.toLowerCase()
);

export const compareTokensForTokensTable = mergeComparators([
compareTokensIcpFirst,
compareTokensWithBalanceFirst,
compareTokensByImportance,
compareTokensAlphabetically,
]);
13 changes: 10 additions & 3 deletions frontend/src/routes/(app)/(nns)/tokens/+page.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import type { Account } from "$lib/types/account";
import { ActionType, type Action } from "$lib/types/actions";
import type { CkBTCAdditionalCanisters } from "$lib/types/ckbtc-canisters";
import type { UserTokenData } from "$lib/types/tokens-page";
import type { UserToken, UserTokenData } from "$lib/types/tokens-page";
import type { Universe, UniverseCanisterIdText } from "$lib/types/universe";
import {
isIcrcTokenUniverse,
Expand All @@ -39,6 +39,7 @@
import { Principal } from "@dfinity/principal";
import { nonNullish } from "@dfinity/utils";
import { onMount } from "svelte";
import { compareTokensForTokensTable } from "$lib/utils/tokens-table.utils";
onMount(() => {
loadCkBTCTokens();
Expand Down Expand Up @@ -195,15 +196,21 @@
}
}
};
const sortTokens = (tokens: UserToken[]) =>
[...tokens].sort(compareTokensForTokensTable);
</script>

<TestIdWrapper testId="tokens-route-component">
{#if $authSignedInStore}
<Tokens userTokensData={$tokensListUserStore} on:nnsAction={handleAction} />
<Tokens
userTokensData={sortTokens($tokensListUserStore)}
on:nnsAction={handleAction}
/>
{:else}
<SignInTokens
on:nnsAction={handleAction}
userTokensData={$tokensListVisitorsStore}
userTokensData={sortTokens($tokensListVisitorsStore)}
/>
{/if}

Expand Down
8 changes: 6 additions & 2 deletions frontend/src/tests/e2e/sns-governance.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ test("Test SNS governance", async ({ page, context }) => {
.getTokensTable()
.getSnsRows();
expect(snsUniverseRows.length).toBeGreaterThanOrEqual(1);
const snsUniverseRow = snsUniverseRows[0];
const snsProjectName = await snsUniverseRow.getProjectName();
const snsProjectName = await snsUniverseRows[0].getProjectName();

// Our first test SNS project is always named "Alfa Centauri".
expect(snsProjectName).toBe("Alfa Centauri");
Expand All @@ -28,6 +27,11 @@ test("Test SNS governance", async ({ page, context }) => {
const askedAmount = 20;
await appPo.getSnsTokens({ amount: askedAmount, name: snsProjectName });

const snsUniverseRow = await appPo
.getTokensPo()
.getTokensPagePo()
.getTokensTable()
.getRowByName(snsProjectName);
expect(await snsUniverseRow.getBalanceNumber()).toEqual(askedAmount);

step("Stake a neuron");
Expand Down
140 changes: 140 additions & 0 deletions frontend/src/tests/lib/utils/tokens-table.utils.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import { CKBTC_UNIVERSE_CANISTER_ID } from "$lib/constants/ckbtc-canister-ids.constants";
import { CKETH_UNIVERSE_CANISTER_ID } from "$lib/constants/cketh-canister-ids.constants";
import { CKUSDC_UNIVERSE_CANISTER_ID } from "$lib/constants/ckusdc-canister-ids.constants";
import {
compareTokensAlphabetically,
compareTokensByImportance,
compareTokensIcpFirst,
compareTokensWithBalanceFirst,
} from "$lib/utils/tokens-table.utils";
import { principal } from "$tests/mocks/sns-projects.mock";
import {
ckBTCTokenBase,
createIcpUserToken,
createUserToken,
createUserTokenLoading,
} from "$tests/mocks/tokens-page.mock";
import { TokenAmountV2 } from "@dfinity/utils";

describe("tokens-table.utils", () => {
const tokenWithBalance = (amount: bigint) =>
createUserToken({
universeId: principal(Number(amount)),
balance: TokenAmountV2.fromUlps({
amount,
token: { name: `T${amount}`, symbol: `T${amount}`, decimals: 8 },
}),
});
const ckBTCToken = createUserToken({
universeId: CKBTC_UNIVERSE_CANISTER_ID,
});
const ckETHTToken = createUserToken({
universeId: CKETH_UNIVERSE_CANISTER_ID,
});
const ckUSDCToken = createUserToken({
universeId: CKUSDC_UNIVERSE_CANISTER_ID,
});

beforeEach(() => {
vi.clearAllMocks();
});

describe("compareTokensIcpFirst", () => {
it("should keep ICP first", () => {
const icpToken = createIcpUserToken();
const ckBTCUserToken = createUserToken(ckBTCTokenBase);

expect(compareTokensIcpFirst(icpToken, ckBTCUserToken)).toEqual(-1);
expect(compareTokensIcpFirst(ckBTCUserToken, icpToken)).toEqual(1);
expect(compareTokensIcpFirst(icpToken, icpToken)).toEqual(0);
});
});

describe("compareTokensWithBalanceFirst", () => {
it("should keep tokens with balance first", () => {
expect(
compareTokensWithBalanceFirst(
tokenWithBalance(1n),
tokenWithBalance(0n)
)
).toEqual(-1);
expect(
compareTokensWithBalanceFirst(
tokenWithBalance(0n),
tokenWithBalance(1n)
)
).toEqual(1);
expect(
compareTokensWithBalanceFirst(
tokenWithBalance(0n),
tokenWithBalance(0n)
)
).toEqual(0);
expect(
compareTokensWithBalanceFirst(
tokenWithBalance(1n),
tokenWithBalance(1n)
)
).toEqual(0);
});

it("should treat token in loading state as having a balance of 0", () => {
expect(
compareTokensWithBalanceFirst(
createUserTokenLoading(),
tokenWithBalance(1n)
)
).toEqual(1);
expect(
compareTokensWithBalanceFirst(
tokenWithBalance(1n),
createUserTokenLoading()
)
).toEqual(-1);
});
});

describe("compareTokensByImportance", () => {
it("should sort tokens by importance", () => {
const token0 = tokenWithBalance(0n);
const expectedOrder = [ckBTCToken, ckETHTToken, ckUSDCToken, token0];
expect(
[token0, ckUSDCToken, ckETHTToken, ckBTCToken].sort(
compareTokensByImportance
)
).toEqual(expectedOrder);
expect(
[ckBTCToken, ckETHTToken, ckUSDCToken, token0].sort(
compareTokensByImportance
)
).toEqual(expectedOrder);
expect(
[ckETHTToken, ckBTCToken, token0, ckUSDCToken].sort(
compareTokensByImportance
)
).toEqual(expectedOrder);
});
});

describe("compareTokensAlphabetically", () => {
const annaToken = createUserToken({ title: "Anna" });
const arnyToken = createUserToken({ title: "Arny" });
const albertTokenLowerCase = createUserToken({ title: "albert" });
const zorroToken = createUserToken({ title: "Zorro" });

it("should sort tokens by importance", () => {
expect(compareTokensAlphabetically(annaToken, arnyToken)).toEqual(-1);
expect(compareTokensAlphabetically(zorroToken, arnyToken)).toEqual(1);
expect(compareTokensAlphabetically(arnyToken, arnyToken)).toEqual(0);
});

it("should sort by titles while ignoring case sensitivity.", () => {
expect(
compareTokensAlphabetically(albertTokenLowerCase, annaToken)
).toEqual(-1);
expect(
compareTokensAlphabetically(annaToken, albertTokenLowerCase)
).toEqual(1);
});
});
});
Loading

0 comments on commit 0994d30

Please sign in to comment.