diff --git a/CHANGELOG-Nns-Dapp-unreleased.md b/CHANGELOG-Nns-Dapp-unreleased.md index 2f5f7a2595e..c18d23492a4 100644 --- a/CHANGELOG-Nns-Dapp-unreleased.md +++ b/CHANGELOG-Nns-Dapp-unreleased.md @@ -38,6 +38,10 @@ proposal is successful, the changes it released will be moved from this file to #### Security +- The "Hide Balance" option now also masks the balances in the accessible names + on the Portfolio page. Before, the cards kept the exact amounts in their + `aria-label` attributes, so a screen reader announced them. + #### Not Published ### Operations diff --git a/frontend/src/lib/components/portfolio/HeldTokensCard.svelte b/frontend/src/lib/components/portfolio/HeldTokensCard.svelte index beec55c260b..60acd6b82c0 100644 --- a/frontend/src/lib/components/portfolio/HeldTokensCard.svelte +++ b/frontend/src/lib/components/portfolio/HeldTokensCard.svelte @@ -6,6 +6,7 @@ import { OWN_CANISTER_ID_TEXT } from "$lib/constants/canister-ids.constants"; import { PRICE_NOT_AVAILABLE_PLACEHOLDER } from "$lib/constants/constants"; import { AppPath } from "$lib/constants/routes.constants"; + import { isBalancePrivacyOptionStore } from "$lib/derived/balance-privacy-active.derived"; import { isDesktopViewportStore } from "$lib/derived/viewport.derived"; import { i18n } from "$lib/stores/i18n"; import type { UserTokenData } from "$lib/types/tokens-page"; @@ -106,7 +107,11 @@ class="balance-usd" data-tid="balance-in-usd" role="cell" - aria-label={`${token.title} USD: ${token?.balanceInUsd ?? 0}`} + aria-label={`${token.title} USD: ${ + $isBalancePrivacyOptionStore + ? $i18n.portfolio.hidden_balance_label + : (token?.balanceInUsd ?? 0) + }`} > $ $ - + {stakedToken.stake.token.symbol} diff --git a/frontend/src/lib/components/portfolio/TokensCardHeader.svelte b/frontend/src/lib/components/portfolio/TokensCardHeader.svelte index 3aa21bf0615..7b6ac2faf52 100644 --- a/frontend/src/lib/components/portfolio/TokensCardHeader.svelte +++ b/frontend/src/lib/components/portfolio/TokensCardHeader.svelte @@ -2,7 +2,9 @@ import PrivacyAwareAmount from "$lib/components/ui/PrivacyAwareAmount.svelte"; import { PRICE_NOT_AVAILABLE_PLACEHOLDER } from "$lib/constants/constants"; import { authSignedInStore } from "$lib/derived/auth.derived"; + import { isBalancePrivacyOptionStore } from "$lib/derived/balance-privacy-active.derived"; import { isMobileViewportStore } from "$lib/derived/viewport.derived"; + import { i18n } from "$lib/stores/i18n"; import { formatCurrencyNumber } from "$lib/utils/format.utils"; import { IconRight } from "@dfinity/gix-components"; import { nonNullish } from "@dfinity/utils"; @@ -32,7 +34,15 @@
{title}
-

+

$

diff --git a/frontend/src/lib/i18n/en.json b/frontend/src/lib/i18n/en.json index 692d2ebad8d..5d1ddfe62d1 100644 --- a/frontend/src/lib/i18n/en.json +++ b/frontend/src/lib/i18n/en.json @@ -1391,6 +1391,7 @@ "login_description": "Sign in and start staking to earn voting rewards and participate in the Internet Computer’s onchain governance.", "no_tokens_card_description": "Store your ICP, ckTokens, and SNS governance tokens safely in the NNS wallet, hosted 100% on the Internet Computer blockchain – sign in to see tokens.", "no_neurons_card_description": "Stake your ICP and SNS tokens in neurons to participate in governance and gain voting rewards – sign in to stake your neurons.", + "hidden_balance_label": "hidden", "held_icp_card_title": "ICP Balance", "held_tokens_card_title": "Tokens Balance", "held_icp_card_link": "View accounts", diff --git a/frontend/src/lib/types/i18n.d.ts b/frontend/src/lib/types/i18n.d.ts index bd13d1e7df7..899915b3487 100644 --- a/frontend/src/lib/types/i18n.d.ts +++ b/frontend/src/lib/types/i18n.d.ts @@ -1464,6 +1464,7 @@ interface I18nPortfolio { login_description: string; no_tokens_card_description: string; no_neurons_card_description: string; + hidden_balance_label: string; held_icp_card_title: string; held_tokens_card_title: string; held_icp_card_link: string; diff --git a/frontend/src/tests/e2e/portfolio-privacy-aria-labels.spec.ts b/frontend/src/tests/e2e/portfolio-privacy-aria-labels.spec.ts new file mode 100644 index 00000000000..b2fd76e9a97 --- /dev/null +++ b/frontend/src/tests/e2e/portfolio-privacy-aria-labels.spec.ts @@ -0,0 +1,152 @@ +/** + * The "Hide Balance" option must keep the amounts out of the accessibility + * tree, and it must still leave every cell with a usable accessible name. + * + * The Portfolio cards mask the amounts on screen with PrivacyAwareAmount. + * PrivacyAwareAmount masks the element content only. An aria-label on the same + * element replaces that content as the accessible name, so the label must + * follow the same store as the visible value. + * + * This test drives the real application and reads the accessible names in both + * states. + */ +import { AppPo } from "$tests/page-objects/App.page-object"; +import { PlaywrightPageObjectElement } from "$tests/page-objects/playwright.page-object"; +import { signInWithNewUser, step } from "$tests/utils/e2e.test-utils"; +import { expect, test } from "@playwright/test"; + +// A Playwright spec cannot import $lib/i18n/en.json, because ESM asks for an +// import attribute of type json. The expected strings are copied here. +const HELD_ICP_CARD_TITLE = "ICP Balance"; +const STAKED_ICP_CARD_TITLE = "ICP Staking Balance"; +const HIDDEN_BALANCE_LABEL = "hidden"; + +const DIGIT = /\d/; + +// playwright.config.ts sets expect.timeout to 0, so every poll needs its own +// timeout. Without one a failing poll hangs until the 300 s test timeout. +const POLL = { timeout: 30_000 }; + +test("Privacy mode keeps the balances out of the Portfolio accessible names", async ({ + page, + browser, +}) => { + await page.goto("/"); + await expect(page).toHaveTitle("Portfolio | Network Nervous System"); + + const pageElement = PlaywrightPageObjectElement.fromPage(page); + const appPo = new AppPo(pageElement); + + step("Sign in"); + await signInWithNewUser({ page, context: browser.contexts()[0] }); + + step("Get some ICP"); + await appPo.getIcpTokens(20); + + step("Stake a neuron, so the staked tokens card has a row"); + await appPo.goToStaking(); + await appPo + .getStakingPo() + .stakeFirstNnsNeuron({ amount: 10, dissolveDelayDays: "max" }); + await appPo.getNeuronsPo().waitFor(); + + step("Open the Portfolio page"); + await page.goto("/"); + const portfolioPagePo = appPo.getPortfolioPo().getPortfolioPagePo(); + const heldCardPo = portfolioPagePo.getHeldICPCardPo(); + const stakedCardPo = portfolioPagePo.getStakedICPCardPo(); + await heldCardPo.waitFor(); + await stakedCardPo.waitFor(); + + // Every accessible name that carries a balance on the Portfolio page. + const getBalanceAriaLabels = async (): Promise<(string | null)[]> => [ + ...(await heldCardPo.getHeldTokensBalanceInUsdAriaLabels()), + await heldCardPo.getAmountAriaLabel(), + ...(await stakedCardPo.getStakedTokensStakeInUsdAriaLabels()), + ...(await stakedCardPo.getStakedTokensStakeInNativeCurrencyAriaLabels()), + await stakedCardPo.getAmountAriaLabel(), + ]; + + // Every attribute inside the two cards that a screen reader can use as an + // accessible name. + const getAllAccessibleNamesInCards = (): Promise => + page + .locator( + [ + '[data-tid="held-icp-card"] [aria-label]', + '[data-tid="held-icp-card"] [title]', + '[data-tid="held-icp-card"] [alt]', + '[data-tid="staked-icp-card"] [aria-label]', + '[data-tid="staked-icp-card"] [title]', + '[data-tid="staked-icp-card"] [alt]', + ].join(", ") + ) + .evaluateAll((elements) => + elements.flatMap((element) => + ["aria-label", "title", "alt"] + .map((attribute) => element.getAttribute(attribute)) + .filter((value): value is string => value !== null) + ) + ); + + const toggleBalancePrivacy = async () => { + // The Popover backdrop closes on click or on Enter/Space, not Escape + // (gix-components handleKeyPress only handles those two keys). The + // backdrop covers the whole screen, including the account-menu button + // itself, so a click on "backdrop" is the reliable way to close it. + const accountMenuPo = appPo.getAccountMenuPo(); + await accountMenuPo.openMenu(); + await accountMenuPo.getToggleBalancePrivacyOptionPo().click(); + await accountMenuPo.click("backdrop"); + await expect.poll(() => accountMenuPo.isOpen(), POLL).toBe(false); + }; + + step("Privacy mode off: every accessible name carries its amount"); + await expect + .poll(getBalanceAriaLabels, POLL) + .toEqual([ + expect.stringMatching(/^Internet Computer USD: \d/), + expect.stringMatching(new RegExp(`^${HELD_ICP_CARD_TITLE}: \\d`)), + expect.stringMatching(/^Internet Computer USD: \d/), + expect.stringMatching(/^Internet Computer ICP: \d/), + expect.stringMatching(new RegExp(`^${STAKED_ICP_CARD_TITLE}: \\d`)), + ]); + + step("Turn privacy mode on"); + await toggleBalancePrivacy(); + + step("Privacy mode on: the amounts are masked on screen"); + await expect.poll(() => heldCardPo.getAmount(), POLL).toContain("•"); + await expect.poll(() => stakedCardPo.getAmount(), POLL).toContain("•"); + + step("Privacy mode on: no accessible name carries an amount"); + // Each name still says which token and which unit the cell holds, so a + // screen reader user can still tell the cells apart. + await expect + .poll(getBalanceAriaLabels, POLL) + .toEqual([ + `Internet Computer USD: ${HIDDEN_BALANCE_LABEL}`, + `${HELD_ICP_CARD_TITLE}: ${HIDDEN_BALANCE_LABEL}`, + `Internet Computer USD: ${HIDDEN_BALANCE_LABEL}`, + `Internet Computer ICP: ${HIDDEN_BALANCE_LABEL}`, + `${STAKED_ICP_CARD_TITLE}: ${HIDDEN_BALANCE_LABEL}`, + ]); + + step("Privacy mode on: no attribute in either card holds a digit"); + const namesWithADigit = (await getAllAccessibleNamesInCards()).filter( + (name) => DIGIT.test(name) + ); + expect(namesWithADigit).toEqual([]); + + step("Turn privacy mode off again: the amounts come back"); + await toggleBalancePrivacy(); + await expect + .poll(getBalanceAriaLabels, POLL) + .toEqual([ + expect.stringMatching(/^Internet Computer USD: \d/), + expect.stringMatching(new RegExp(`^${HELD_ICP_CARD_TITLE}: \\d`)), + expect.stringMatching(/^Internet Computer USD: \d/), + expect.stringMatching(/^Internet Computer ICP: \d/), + expect.stringMatching(new RegExp(`^${STAKED_ICP_CARD_TITLE}: \\d`)), + ]); +}); diff --git a/frontend/src/tests/lib/components/portfolio/HeldTokensCard.spec.ts b/frontend/src/tests/lib/components/portfolio/HeldTokensCard.spec.ts index 9723a912e4c..ee29dea913b 100644 --- a/frontend/src/tests/lib/components/portfolio/HeldTokensCard.spec.ts +++ b/frontend/src/tests/lib/components/portfolio/HeldTokensCard.spec.ts @@ -130,6 +130,36 @@ describe("HeldTokensCard", () => { expect(nativeBalances).toEqual(["••• ICP", "••• ckBTC", "••• ckETH"]); }); + it("should show the balances in the aria labels when privacy mode is off", async () => { + const po = renderComponent({ + topHeldTokens: mockTokens, + usdAmount: 6000, + }); + + expect(await po.getHeldTokensBalanceInUsdAriaLabels()).toEqual([ + "Internet Computer USD: 100", + "ckBTC USD: 200", + "ckETH USD: 300", + ]); + expect(await po.getAmountAriaLabel()).toBe("Tokens Balance: 6000"); + }); + + it("should not expose the balances in the aria labels when privacy mode is on", async () => { + balancePrivacyOptionStore.set("hide"); + + const po = renderComponent({ + topHeldTokens: mockTokens, + usdAmount: 6000, + }); + + expect(await po.getHeldTokensBalanceInUsdAriaLabels()).toEqual([ + "Internet Computer USD: hidden", + "ckBTC USD: hidden", + "ckETH USD: hidden", + ]); + expect(await po.getAmountAriaLabel()).toBe("Tokens Balance: hidden"); + }); + it("should render links for each row", async () => { const po = renderComponent({ topHeldTokens: mockTokens, diff --git a/frontend/src/tests/lib/components/portfolio/StakedTokensCard.spec.ts b/frontend/src/tests/lib/components/portfolio/StakedTokensCard.spec.ts index c72da8c22f0..188409e2429 100644 --- a/frontend/src/tests/lib/components/portfolio/StakedTokensCard.spec.ts +++ b/frontend/src/tests/lib/components/portfolio/StakedTokensCard.spec.ts @@ -177,6 +177,50 @@ describe("StakedTokensCard", () => { ]); }); + it("should show the balances in the aria labels when privacy mode is off", async () => { + const po = renderComponent({ + topStakedTokens: mockStakedTokens, + usdAmount: 5000, + }); + + expect(await po.getStakedTokensStakeInUsdAriaLabels()).toEqual([ + "Internet Computer USD: 100", + "Project 1 USD: 200", + "Project 2 USD: 300", + "Project 3 USD: 400", + ]); + expect(await po.getStakedTokensStakeInNativeCurrencyAriaLabels()).toEqual([ + "Internet Computer ICP: 0.01", + "Project 1 TET: 0.01", + "Project 2 TET: 0.01", + "Project 3 TET: 0.01", + ]); + expect(await po.getAmountAriaLabel()).toBe("Staking Balance: 5000"); + }); + + it("should not expose the balances in the aria labels when privacy mode is on", async () => { + balancePrivacyOptionStore.set("hide"); + + const po = renderComponent({ + topStakedTokens: mockStakedTokens, + usdAmount: 5000, + }); + + expect(await po.getStakedTokensStakeInUsdAriaLabels()).toEqual([ + "Internet Computer USD: hidden", + "Project 1 USD: hidden", + "Project 2 USD: hidden", + "Project 3 USD: hidden", + ]); + expect(await po.getStakedTokensStakeInNativeCurrencyAriaLabels()).toEqual([ + "Internet Computer ICP: hidden", + "Project 1 TET: hidden", + "Project 2 TET: hidden", + "Project 3 TET: hidden", + ]); + expect(await po.getAmountAriaLabel()).toBe("Staking Balance: hidden"); + }); + it("should not show info row when numberOfTopHeldTokens is the same as the number of topStakedTokens", async () => { const po = renderComponent({ topStakedTokens: mockStakedTokens.slice(0, 3), diff --git a/frontend/src/tests/page-objects/HeldTokensCard.page-object.ts b/frontend/src/tests/page-objects/HeldTokensCard.page-object.ts index 640802f5fb7..ad398d9fd48 100644 --- a/frontend/src/tests/page-objects/HeldTokensCard.page-object.ts +++ b/frontend/src/tests/page-objects/HeldTokensCard.page-object.ts @@ -30,6 +30,10 @@ class HeldTokensCardRowPo extends BasePageObject { getHeldTokenBalanceInUsd(): Promise { return this.getText("balance-in-usd"); } + + getHeldTokenBalanceInUsdAriaLabel(): Promise { + return this.getElement("balance-in-usd").getAttribute("aria-label"); + } } export class HeldTokensCardPo extends BasePageObject { @@ -55,6 +59,10 @@ export class HeldTokensCardPo extends BasePageObject { return this.getText("amount"); } + getAmountAriaLabel(): Promise { + return this.getElement("amount").getAttribute("aria-label"); + } + getInfoRow(): PageObjectElement { return this.getElement("info-row"); } @@ -74,6 +82,14 @@ export class HeldTokensCardPo extends BasePageObject { return Promise.all(rows.map((row) => row.getHeldTokenBalanceInUsd())); } + async getHeldTokensBalanceInUsdAriaLabels(): Promise<(string | null)[]> { + const rows = await this.getRows(); + + return Promise.all( + rows.map((row) => row.getHeldTokenBalanceInUsdAriaLabel()) + ); + } + async getHeldTokensBalanceInNativeCurrency(): Promise { const rows = await this.getRows(); diff --git a/frontend/src/tests/page-objects/StakedTokensCard.page-object.ts b/frontend/src/tests/page-objects/StakedTokensCard.page-object.ts index 84c2fc4caee..233836f717a 100644 --- a/frontend/src/tests/page-objects/StakedTokensCard.page-object.ts +++ b/frontend/src/tests/page-objects/StakedTokensCard.page-object.ts @@ -44,6 +44,14 @@ class StakedTokensCardRowPo extends BasePageObject { getStakedTokenStakeInNativeCurrency(): Promise { return this.getTextWithCollapsedWhitespaces("stake-in-native"); } + + getStakedTokenStakeInUsdAriaLabel(): Promise { + return this.getElement("stake-in-usd").getAttribute("aria-label"); + } + + getStakedTokenStakeInNativeCurrencyAriaLabel(): Promise { + return this.getElement("stake-in-native").getAttribute("aria-label"); + } } export class StakedTokensCardPo extends BasePageObject { @@ -69,6 +77,10 @@ export class StakedTokensCardPo extends BasePageObject { return this.getText("amount"); } + getAmountAriaLabel(): Promise { + return this.getElement("amount").getAttribute("aria-label"); + } + getInfoRow(): PageObjectElement { return this.getElement("info-row"); } @@ -108,6 +120,22 @@ export class StakedTokensCardPo extends BasePageObject { ); } + async getStakedTokensStakeInUsdAriaLabels(): Promise<(string | null)[]> { + const rows = await this.getRows(); + return Promise.all( + rows.map((row) => row.getStakedTokenStakeInUsdAriaLabel()) + ); + } + + async getStakedTokensStakeInNativeCurrencyAriaLabels(): Promise< + (string | null)[] + > { + const rows = await this.getRows(); + return Promise.all( + rows.map((row) => row.getStakedTokenStakeInNativeCurrencyAriaLabel()) + ); + } + async getRowsTags(): Promise { const rows = await this.getRows(); return Promise.all(rows.map((row) => row.getRowTag()));