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: add count up #1642

Merged
merged 6 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
"memoize-one": "^6.0.0",
"qrcode.react": "^3.0.2",
"react-chartjs-2": "^5.2.0",
"react-countup": "^6.5.3",
"react-json-to-table": "^0.1.7",
"react-markdown": "^8.0.7",
"react-tooltip": "^4.2.21",
Expand Down
16 changes: 14 additions & 2 deletions packages/extension-polkagate/src/components/FormatPrice.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { BN } from '@polkadot/util';

import { Grid, Skeleton, Typography } from '@mui/material';
import React, { useMemo } from 'react';
import CountUp from 'react-countup';

import { useCurrency } from '../hooks';
import { ASSETS_AS_CURRENCY_LIST } from '../util/currencyList';
Expand All @@ -27,6 +28,7 @@ interface Props {
textColor?: string;
height?: number;
width?: string;
withCountUp?: boolean;
}

export function nFormatter (num: number, decimalPoint: number) {
Expand Down Expand Up @@ -54,7 +56,7 @@ export function nFormatter (num: number, decimalPoint: number) {

const DECIMAL_POINTS_FOR_CRYPTO_AS_CURRENCY = 4;

function FormatPrice ({ amount, commify, decimalPoint = 2, decimals, fontSize, fontWeight, height, lineHeight = 1, mt = '0px', num, price, sign, skeletonHeight = 15, textAlign = 'left', textColor, width = '90px' }: Props): React.ReactElement<Props> {
function FormatPrice ({ amount, commify, decimalPoint = 2, decimals, fontSize, fontWeight, height, lineHeight = 1, mt = '0px', num, price, sign, skeletonHeight = 15, textAlign = 'left', textColor, width = '90px', withCountUp }: Props): React.ReactElement<Props> {
const currency = useCurrency();

const total = useMemo(() => {
Expand Down Expand Up @@ -91,7 +93,17 @@ function FormatPrice ({ amount, commify, decimalPoint = 2, decimals, fontSize, f
lineHeight={lineHeight}
sx={{ color: textColor }}
>
{sign || currency?.sign || ''}{ commify ? fixFloatingPoint(total as number, _decimalPoint, true) : nFormatter(total as number, _decimalPoint)}
{withCountUp
? <CountUp
decimals={_decimalPoint}
duration={1}
end={parseFloat(String(total))}
prefix={sign || currency?.sign || ''}
/>
Comment on lines +97 to +102
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Based on the gathered information, I can now provide the final response regarding the accessibility concerns of the CountUp component.

Add accessibility attributes to CountUp implementation

The current CountUp implementation lacks proper accessibility considerations. While the codebase shows awareness of reduced motion preferences in other components, the numeric animation needs similar treatment. Please update the implementation to:

  1. Wrap CountUp in a container with appropriate ARIA attributes:
<span aria-atomic="true" aria-live="polite">
  <CountUp
    decimals={_decimalPoint}
    duration={prefersReducedMotion ? 0 : 1}
    end={parseFloat(String(total))}
    prefix={sign || currency?.sign || ''}
  />
</span>
  1. Respect user's motion preferences by utilizing the existing prefersReducedMotion check from useAnimateOnce.ts

This will ensure:

  • Screen readers announce the final value properly
  • Users with motion sensitivity preferences are respected
  • The numeric value remains accessible during animation
🔗 Analysis chain

Verify accessibility impact

Please ensure that the CountUp animation:

  1. Respects user's reduced motion preferences
  2. Is properly announced by screen readers
  3. Doesn't cause content layout shifts

Let me check the content of these files to understand the existing accessibility implementations.


Let me check if there are any existing ARIA attributes or accessibility considerations in the FormatPrice component, and also check the react-countup package usage.


Let me check the TotalBalancePieChart component to see how CountUp is implemented there, and also verify the package.json for the react-countup version and any related accessibility packages.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for existing accessibility considerations in the codebase
rg -l 'prefers-reduced-motion|aria-live|aria-atomic' 

Length of output: 170


Script:

#!/bin/bash
# Check content of files with accessibility considerations
echo "=== VaadinIcon.tsx ==="
cat packages/extension-polkagate/src/components/VaadinIcon.tsx

echo -e "\n=== useAnimateOnce.ts ==="
cat packages/extension-polkagate/src/hooks/useAnimateOnce.ts

# Also check if CountUp component has any accessibility considerations
rg -A 5 "CountUp" packages/extension-polkagate/src/components/FormatPrice.tsx

Length of output: 4728


Script:

#!/bin/bash
# Check for any ARIA attributes in FormatPrice
rg -A 2 'aria-|role=' packages/extension-polkagate/src/components/FormatPrice.tsx

# Check if react-countup is used elsewhere with accessibility considerations
rg -l "react-countup"

# Look for any test files related to FormatPrice
fd "FormatPrice.*.test" || fd "FormatPrice.*.spec"

Length of output: 307


Script:

#!/bin/bash
# Check CountUp implementation in TotalBalancePieChart
cat packages/extension-polkagate/src/fullscreen/homeFullScreen/partials/TotalBalancePieChart.tsx

# Check package.json for react-countup version and accessibility-related packages
cat package.json | grep -A 5 -B 5 "react-countup"

Length of output: 11484

: <>
{sign || currency?.sign || ''}{ commify ? fixFloatingPoint(total as number, _decimalPoint, true) : nFormatter(total as number, _decimalPoint)}
</>
}
</Typography>
: <Skeleton
animation='wave'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type { FetchedBalance } from '../../../hooks/useAssetsBalances';
import { ArrowDropDown as ArrowDropDownIcon } from '@mui/icons-material';
import { Box, Collapse, Divider, Grid, type Theme, Typography, useTheme } from '@mui/material';
import React, { useCallback, useContext, useEffect, useMemo, useState } from 'react';
import CountUp from 'react-countup';

import { BN, BN_ZERO } from '@polkadot/util';

Expand All @@ -19,7 +20,7 @@ import { isPriceOutdated } from '../../../popup/home/YouHave';
import { COIN_GECKO_PRICE_CHANGE_DURATION } from '../../../util/api/getPrices';
import { DEFAULT_COLOR, TEST_NETS, TOKENS_WITH_BLACK_LOGO } from '../../../util/constants';
import getLogo2 from '../../../util/getLogo2';
import { amountToHuman, fixFloatingPoint } from '../../../util/utils';
import { amountToHuman, countDecimalPlaces, fixFloatingPoint } from '../../../util/utils';
import Chart from './Chart';

interface Props {
Expand Down Expand Up @@ -194,6 +195,20 @@ function TotalBalancePieChart ({ hideNumbers, setGroupedAssets }: Props): React.

const toggleAssets = useCallback(() => setShowMore(!showMore), [showMore]);

const portfolioChange = useMemo(() => {
if (!youHave?.change) {
return 0;
}

const value = fixFloatingPoint(youHave.change, 2, false, true);

return parseFloat(value);
}, [youHave?.change]);

const changeSign = !youHave?.change
? ''
: youHave.change > 0 ? '+ ' : '- ';

return (
<Grid alignItems='flex-start' container direction='column' item justifyContent='flex-start' sx={{ bgcolor: 'background.paper', borderRadius: '5px', boxShadow: '2px 3px 4px 0px rgba(0, 0, 0, 0.1)', minHeight: '287px', p: '15px 25px 10px', width: '430px' }}>
<Grid alignItems='flex-start' container item justifyContent='flex-start'>
Expand All @@ -214,9 +229,16 @@ function TotalBalancePieChart ({ hideNumbers, setGroupedAssets }: Props): React.
fontWeight={700}
num={youHave?.portfolio}
textColor= { isPriceOutdated(youHave) ? 'primary.light' : 'text.primary'}
withCountUp
/>
<Typography sx={{ color: youHave.change > 0 ? 'success.main' : 'warning.main', fontSize: '18px', fontWeight: 500 }}>
{youHave.change > 0 ? '+ ' : '- '}{currency?.sign}{ fixFloatingPoint(youHave?.change, 2, true, true)} {`(${COIN_GECKO_PRICE_CHANGE_DURATION}h)`}
<Typography sx={{ color: !youHave.change ? 'secondary.contrastText' : youHave.change > 0 ? 'success.main' : 'warning.main', fontSize: '18px', fontWeight: 500 }}>
<CountUp
decimals={countDecimalPlaces(portfolioChange) || 2}
duration={1}
end={portfolioChange}
prefix={`${changeSign}${currency?.sign}`}
suffix={`(${COIN_GECKO_PRICE_CHANGE_DURATION}h)`}
/>
</Typography>
</>
}
Expand Down
10 changes: 7 additions & 3 deletions packages/extension-polkagate/src/hooks/useYouHave.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ import type { BN } from '@polkadot/util';
import { useCallback, useContext, useMemo } from 'react';

import { AccountsAssetsContext } from '../components';
import { ASSETS_AS_CURRENCY_LIST } from '../util/currencyList';
import { amountToHuman } from '../util/utils';
import { usePrices } from '.';
import { useCurrency, usePrices } from '.';

export interface YouHaveType {
change: number;
Expand All @@ -23,6 +24,7 @@ export interface YouHaveType {
export default function useYouHave (): YouHaveType | undefined | null {
const pricesInCurrencies = usePrices();
const { accountsAssets } = useContext(AccountsAssetsContext);
const currency = useCurrency();

const calcPrice = useCallback(
(assetPrice: number | undefined, balance: BN, decimal: number) =>
Expand Down Expand Up @@ -62,10 +64,12 @@ export default function useYouHave (): YouHaveType | undefined | null {
});
});

const change = totalPrice - totalBeforeChange;
const change = currency?.code
? ASSETS_AS_CURRENCY_LIST.includes(currency.code.toUpperCase()) ? 0 : totalPrice - totalBeforeChange
: 0;

return { change, date, portfolio: totalPrice } as unknown as YouHaveType;
}, [accountsAssets, calcChange, calcPrice, pricesInCurrencies]);
}, [accountsAssets, calcChange, calcPrice, currency, pricesInCurrencies]);

return youHave;
}
3 changes: 2 additions & 1 deletion packages/extension-polkagate/src/popup/home/YouHave.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,14 @@ export default function YouHave ({ hideNumbers, setHideNumbers }: Props): React.
/>
: <Grid item pr='15px'>
<FormatPrice
fontSize='38px'
fontSize='32px'
fontWeight={500}
height={36}
num={youHave?.portfolio }
skeletonHeight={36}
textColor= { isPriceOutdated(youHave) ? 'primary.light' : 'text.primary'}
width='223px'
withCountUp
/>
</Grid>
}
Expand Down
6 changes: 6 additions & 0 deletions packages/extension-polkagate/src/util/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ function countLeadingZerosInFraction (numStr: string) {
return 0;
}

export function countDecimalPlaces (n: number) {
const match = n.toString().match(/\.(\d+)/);

return match ? match[1].length : 0;
}
Comment on lines +53 to +57
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add type safety and handle edge cases.

The function implementation could be improved with better type safety and edge case handling:

-export function countDecimalPlaces (n: number) {
+export function countDecimalPlaces (n: number): number {
+  if (!Number.isFinite(n)) {
+    return 0;
+  }
+
+  // Handle scientific notation
+  const str = n.toString().toLowerCase();
+  if (str.includes('e')) {
+    const [_, exp] = str.split('e');
+    return Math.max(0, -Number(exp));
+  }
+
   const match = n.toString().match(/\.(\d+)/);
 
   return match ? match[1].length : 0;
 }

Also consider adding JSDoc documentation to describe the function's purpose and behavior:

/**
 * Counts the number of decimal places in a given number.
 * @param n - The number to analyze
 * @returns The count of decimal places (0 for integers, NaN, or Infinity)
 * @example
 * countDecimalPlaces(1.23) // returns 2
 * countDecimalPlaces(1) // returns 0
 * countDecimalPlaces(1e-10) // returns 10
 */


export function fixFloatingPoint (_number: number | string, decimalDigit = FLOATING_POINT_DIGIT, commify?: boolean, dynamicDecimal?: boolean): string {
const MAX_DECIMAL_POINTS = 6;

Expand Down
19 changes: 19 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -8297,6 +8297,13 @@ __metadata:
languageName: node
linkType: hard

"countup.js@npm:^2.8.0":
version: 2.8.0
resolution: "countup.js@npm:2.8.0"
checksum: 10/6681de9de9acd0d65b91183af280a462f2fbbc19ac5a90d30ea94e388f12b0d1cc7fa2b63b6bb83adc9dc38f938cd2a41f727a0b2ae7edf0d82f4b2ccb51e98c
languageName: node
linkType: hard

"crc-32@npm:^1.2.2":
version: 1.2.2
resolution: "crc-32@npm:1.2.2"
Expand Down Expand Up @@ -16694,6 +16701,17 @@ __metadata:
languageName: node
linkType: hard

"react-countup@npm:^6.5.3":
version: 6.5.3
resolution: "react-countup@npm:6.5.3"
dependencies:
countup.js: "npm:^2.8.0"
peerDependencies:
react: ">= 16.3.0"
checksum: 10/5699e475bdd82b0b100174acbe24a33c9025080bacbba9f28593308145eacfb3e0d30be5de5bb153121346b719375e317a9f2aea87776f38d508cbe769ac0d8b
languageName: node
linkType: hard

"react-dom@npm:^16.7.0":
version: 16.14.0
resolution: "react-dom@npm:16.14.0"
Expand Down Expand Up @@ -17620,6 +17638,7 @@ __metadata:
pinst: "npm:^3.0.0"
qrcode.react: "npm:^3.0.2"
react-chartjs-2: "npm:^5.2.0"
react-countup: "npm:^6.5.3"
react-json-to-table: "npm:^0.1.7"
react-markdown: "npm:^8.0.7"
react-tooltip: "npm:^4.2.21"
Expand Down
Loading