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

style: shake account icons once on activation #1603

Merged
merged 3 commits into from
Oct 24, 2024
Merged

style: shake account icons once on activation #1603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
67502d2
style: shake account icons once on activation
Nick-1979 Oct 23, 2024
cf333b9
refactor: add accesibility feature
Nick-1979 Oct 23, 2024
b5d7a97
refactor: optimize useAnimateOnce (#1604)
AMIRKHANEF Oct 24, 2024
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
39 changes: 26 additions & 13 deletions packages/extension-polkagate/src/fullscreen/accountDetails/components/AccountIconsFs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,21 @@
// Copyright 2019-2024 @polkadot/extension-polkagate authors & contributors
// SPDX-License-Identifier: Apache-2.0
// @ts-nocheck

/* eslint-disable react/jsx-max-props-per-line */

import type { DeriveAccountInfo } from '@polkadot/api-derive/types';
import type { Option, u128, Vec } from '@polkadot/types';
//@ts-ignore
import type { PalletProxyAnnouncement, PalletRecoveryActiveRecovery } from '@polkadot/types/lookup';
Nick-1979 marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +6 to +9
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix type shadowing and improve type imports.

The imported Proxy type shadows the global Proxy object, which could lead to confusion. Consider renaming it to be more specific.

-import type { Proxy } from '../../../util/types';
+import type { AccountProxy } from '../../../util/types';

// Update usage of Proxy type to AccountProxy throughout the file
-const fetchedProxies = JSON.parse(JSON.stringify(_p[0])) as unknown as Proxy[];
+const fetchedProxies = JSON.parse(JSON.stringify(_p[0])) as unknown as AccountProxy[];

Committable suggestion was skipped due to low confidence.

import type { Proxy } from '../../../util/types';

import { faChain, faCheckCircle, faCircleInfo, faShieldHalved, faSitemap } from '@fortawesome/free-solid-svg-icons';
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
import { Grid, IconButton, useTheme } from '@mui/material';
import React, { useCallback, useContext, useEffect, useMemo, useState } from 'react';

import { DeriveAccountInfo } from '@polkadot/api-derive/types';

import { ActionContext, Infotip } from '../../../components';
import { useInfo, useTranslation } from '../../../hooks';
import { useAnimateOnce, useInfo, useTranslation } from '../../../hooks';
import { windowOpen } from '../../../messaging';
import { IDENTITY_CHAINS, PROXY_CHAINS, SOCIAL_RECOVERY_CHAINS } from '../../../util/constants';

Expand All @@ -23,16 +24,21 @@ interface AddressDetailsProps {
accountInfo: DeriveAccountInfo | undefined | null
}

export default function AccountIconsFs({ accountInfo, address }: AddressDetailsProps): React.ReactElement {
function AccountIconsFs ({ accountInfo, address }: AddressDetailsProps): React.ReactElement {
const { t } = useTranslation();
const theme = useTheme();

const onAction = useContext(ActionContext);
const { account, api, chain, formatted } = useInfo(address);

const [hasID, setHasID] = useState<boolean | undefined>();
const [isRecoverable, setIsRecoverable] = useState<boolean | undefined>();
const [hasProxy, setHasProxy] = useState<boolean | undefined>();

const shakeProxy = useAnimateOnce(hasProxy);
const shakeIdentity = useAnimateOnce(hasID);
const shakeShield = useAnimateOnce(isRecoverable);

const identityToolTipTxt = useMemo(() => {
if (!chain) {
return 'Account is in Any Chain mode';
Expand Down Expand Up @@ -93,19 +99,20 @@ export default function AccountIconsFs({ accountInfo, address }: AddressDetailsP
setHasID(false);
}

if (api.query?.recovery && SOCIAL_RECOVERY_CHAINS.includes(account.genesisHash)) {
api.query.recovery.recoverable(formatted)
if (api.query?.['recovery'] && SOCIAL_RECOVERY_CHAINS.includes(account.genesisHash)) {
api.query['recovery']['recoverable'](formatted)
.then((r) =>
setIsRecoverable(r.isSome))
setIsRecoverable((r as Option<PalletRecoveryActiveRecovery>).isSome))
.catch(console.error);
} else {
setIsRecoverable(false);
}

if (api.query?.proxy && PROXY_CHAINS.includes(account.genesisHash)) {
api.query.proxy.proxies(formatted)
if (api.query?.['proxy'] && PROXY_CHAINS.includes(account.genesisHash)) {
api.query['proxy']['proxies'](formatted)
.then((p) => {
const fetchedProxies = JSON.parse(JSON.stringify(p[0])) as unknown as Proxy[];
const _p = p as unknown as [Vec<PalletProxyAnnouncement>, u128];
const fetchedProxies = JSON.parse(JSON.stringify(_p[0])) as unknown as Proxy[];

setHasProxy(fetchedProxies.length > 0);
}).catch(console.error);
Expand All @@ -115,11 +122,11 @@ export default function AccountIconsFs({ accountInfo, address }: AddressDetailsP
}, [api, address, formatted, account?.genesisHash, accountInfo]);

const openIdentity = useCallback(() => {
address && chain && windowOpen(`/manageIdentity/${address}`);
address && chain && windowOpen(`/manageIdentity/${address}`).catch(console.error);
Nick-1979 marked this conversation as resolved.
Show resolved Hide resolved
}, [address, chain]);

const openSocialRecovery = useCallback(() => {
address && chain && windowOpen(`/socialRecovery/${address}/false`);
address && chain && windowOpen(`/socialRecovery/${address}/false`).catch(console.error);
}, [address, chain]);

const openManageProxy = useCallback(() => {
Expand All @@ -134,10 +141,12 @@ export default function AccountIconsFs({ accountInfo, address }: AddressDetailsP
? accountInfo?.identity?.displayParent
? <FontAwesomeIcon
icon={faChain}
shake={shakeIdentity}
Nick-1979 marked this conversation as resolved.
Show resolved Hide resolved
style={{ border: '1px solid', borderRadius: '5px', color: theme.palette.success.main, fontSize: '13px', padding: '2px' }}
/>
: <FontAwesomeIcon
icon={faCheckCircle}
shake={shakeIdentity}
style={{ border: '1px solid', borderRadius: '5px', color: theme.palette.success.main, fontSize: '16px', padding: '2px' }}
/>
: <FontAwesomeIcon
Expand All @@ -155,6 +164,7 @@ export default function AccountIconsFs({ accountInfo, address }: AddressDetailsP
>
<FontAwesomeIcon
icon={faShieldHalved}
shake={shakeShield}
style={{ border: '1px solid', borderRadius: '5px', color: isRecoverable ? theme.palette.success.main : theme.palette.action.disabledBackground, fontSize: '16px', padding: '2px' }}
/>
</IconButton>
Expand All @@ -165,6 +175,7 @@ export default function AccountIconsFs({ accountInfo, address }: AddressDetailsP
<IconButton onClick={openManageProxy} sx={{ height: '16px', width: '16px' }}>
<FontAwesomeIcon
icon={faSitemap}
shake={shakeProxy}
style={{ border: '1px solid', borderRadius: '5px', color: hasProxy ? theme.palette.success.main : theme.palette.action.disabledBackground, fontSize: '16px', padding: '2px' }}
/>
</IconButton>
Expand All @@ -173,3 +184,5 @@ export default function AccountIconsFs({ accountInfo, address }: AddressDetailsP
</Grid>
);
}

export default React.memo(AccountIconsFs);
1 change: 1 addition & 0 deletions packages/extension-polkagate/src/hooks/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export { default as useActiveEraIndex } from './useActiveEraIndex';
export { default as useActiveRecoveries } from './useActiveRecoveries';
export { default as useActiveValidators } from './useActiveValidators';
export { default as useAlerts } from './useAlerts';
export { default as useAnimateOnce } from './useAnimateOnce';
export { default as useApi } from './useApi';
export { default as useApiWithChain } from './useApiWithChain';
export { default as useApiWithChain2 } from './useApiWithChain2';
Expand Down
62 changes: 62 additions & 0 deletions packages/extension-polkagate/src/hooks/useAnimateOnce.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright 2019-2024 @polkadot/extension-ui authors & contributors
// SPDX-License-Identifier: Apache-2.0

import { useCallback, useEffect, useRef, useState } from 'react';

interface AnimateOnceConfig {
duration?: number;
delay?: number;
onStart?: () => void;
onComplete?: () => void;
}

const DEFAULT_DURATION = 500;
const DEFAULT_DELAY = 0;

type Timer = ReturnType<typeof setTimeout>;

/**
* A hook that triggers a one-time animation state when a condition becomes true.
* Accounts for 'prefers-reduced-motion' settings.
* @param condition - The condition that triggers the animation
* @param config - Configuration options for the animation
* @returns animate: boolean - Current animation state
*/
export default function useAnimateOnce (condition: boolean | undefined, config: AnimateOnceConfig = {}): boolean {
const [animate, setAnimate] = useState(false);
Comment on lines +25 to +26
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

Consider stricter typing for the condition parameter.

The condition parameter accepts undefined, which might lead to unexpected behavior. Consider making it strictly boolean.

-export default function useAnimateOnce (condition: boolean | undefined, config: AnimateOnceConfig = {}): boolean {
+export default function useAnimateOnce (condition: boolean, config: AnimateOnceConfig = {}): boolean {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export default function useAnimateOnce (condition: boolean | undefined, config: AnimateOnceConfig = {}): boolean {
const [animate, setAnimate] = useState(false);
export default function useAnimateOnce (condition: boolean, config: AnimateOnceConfig = {}): boolean {
const [animate, setAnimate] = useState(false);

const timeoutRef = useRef<Timer>();

const { delay = DEFAULT_DELAY, duration = DEFAULT_DURATION, onComplete, onStart } = config;

// Check if the user prefers reduced motion
const prefersReducedMotion = window.matchMedia('(prefers-reduced-motion: reduce)').matches;
Comment on lines +31 to +32
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

Enhance accessibility check implementation.

The current implementation checks motion preferences only once during initialization. Consider using a more robust solution that can react to preference changes.

-const prefersReducedMotion = window.matchMedia('(prefers-reduced-motion: reduce)').matches;
+const mediaQuery = window.matchMedia('(prefers-reduced-motion: reduce)');
+const [prefersReducedMotion, setPrefersReducedMotion] = useState(mediaQuery.matches);
+
+useEffect(() => {
+  const handler = (event: MediaQueryListEvent) => {
+    setPrefersReducedMotion(event.matches);
+  };
+  
+  mediaQuery.addEventListener('change', handler);
+  return () => mediaQuery.removeEventListener('change', handler);
+}, []);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Check if the user prefers reduced motion
const prefersReducedMotion = window.matchMedia('(prefers-reduced-motion: reduce)').matches;
// Check if the user prefers reduced motion
const mediaQuery = window.matchMedia('(prefers-reduced-motion: reduce)');
const [prefersReducedMotion, setPrefersReducedMotion] = useState(mediaQuery.matches);
useEffect(() => {
const handler = (event: MediaQueryListEvent) => {
setPrefersReducedMotion(event.matches);
};
mediaQuery.addEventListener('change', handler);
return () => mediaQuery.removeEventListener('change', handler);
}, []);


// Cleanup function to clear any pending timeouts
const cleanup = useCallback(() => {
if (timeoutRef.current) {
clearTimeout(timeoutRef.current);
timeoutRef.current = undefined;
}
}, []);
Comment on lines +35 to +40
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

Enhance cleanup implementation.

The cleanup function should also reset the animation state to ensure consistent behavior during unmount or when dependencies change.

 const cleanup = useCallback(() => {
   if (timeoutRef.current) {
     clearTimeout(timeoutRef.current);
     timeoutRef.current = undefined;
+    setAnimate(false);
   }
 }, []);

Committable suggestion was skipped due to low confidence.


const trigger = useCallback(() => {
cleanup();
onStart?.();
setAnimate(true);

timeoutRef.current = setTimeout(() => {
setAnimate(false);
onComplete?.();
}, duration);
}, [cleanup, duration, onStart, onComplete]);

useEffect(() => {
if (condition && !prefersReducedMotion) {
timeoutRef.current = setTimeout(trigger, delay);
}

return cleanup;
}, [cleanup, condition, delay, trigger, prefersReducedMotion]);

return animate;
}
3 changes: 2 additions & 1 deletion packages/extension/public/locales/en/translation.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1401,5 +1401,6 @@
"Invalid endpoint format!": "",
"The chain already exists, no need to add it again!": "",
"Click to hide all the profile accounts from websites!": "",
"Checking": ""
"Checking": "",
"Comments ({{count}})": ""
}
Loading