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 wallet amount in dollar #921

Closed
Closed
31 changes: 31 additions & 0 deletions components/UI/ShowHideAddressProps.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import React, { useState } from 'react';
import EyeIcon from "@components/UI/iconsComponents/icons/eyeIcon";
import EyeOffIcon from './iconsComponents/icons/eyeOffIcon';

type ToggleVisibilityProps = {
address: string;
className?: string;
iconSize?: string;
wallet:boolean;
hideBalance: boolean;
setHideBalance: React.Dispatch<React.SetStateAction<boolean>>;
}
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

Improve type definition structure and remove unused props.

The type definition needs the following improvements:

  1. Remove the unused address prop
  2. Fix inconsistent spacing
  3. Add proper type annotation for the wallet prop default value

Apply this diff:

-type  ToggleVisibilityProps = {
-  address: string;
-  className?: string;
-  iconSize?: string;
-  wallet:boolean;
-  hideBalance: boolean;
-  setHideBalance: React.Dispatch<React.SetStateAction<boolean>>;
-}
+type ToggleVisibilityProps = {
+  className?: string;
+  iconSize?: string;
+  wallet?: boolean;
+  hideBalance: boolean;
+  setHideBalance: React.Dispatch<React.SetStateAction<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
type ToggleVisibilityProps = {
address: string;
className?: string;
iconSize?: string;
wallet:boolean;
hideBalance: boolean;
setHideBalance: React.Dispatch<React.SetStateAction<boolean>>;
}
type ToggleVisibilityProps = {
className?: string;
iconSize?: string;
wallet?: boolean;
hideBalance: boolean;
setHideBalance: React.Dispatch<React.SetStateAction<boolean>>;
};


const ToggleVisibility: React.FC< ToggleVisibilityProps> = ({ address, className, iconSize = "24",wallet=false, hideBalance, setHideBalance }) => {
const toggleBalance = (e: React.MouseEvent<HTMLButtonElement>) => {
e.stopPropagation();
setHideBalance(!hideBalance);
};

return (
<button className={className} onClick={toggleBalance}>
{!hideBalance ? (
<EyeIcon width={iconSize} color={wallet ? undefined : "#F4FAFF"} />
) : (
<EyeOffIcon width={iconSize} color={wallet ? undefined : "#F4FAFF"}/>
)}
</button>
);
};
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 component implementation with accessibility and error handling.

The component needs several improvements for better accessibility and robustness:

  1. Add proper ARIA attributes
  2. Add error handling
  3. Fix inconsistent spacing
  4. Consider renaming component to better reflect its purpose

Apply these improvements:

-const  ToggleVisibility: React.FC< ToggleVisibilityProps> = ({ address, className, iconSize = "24",wallet=false, hideBalance, setHideBalance }) => {
-    const toggleBalance = (e: React.MouseEvent<HTMLButtonElement>) => {
-      e.stopPropagation();
-      setHideBalance(!hideBalance);
-    };
+const ToggleVisibility: React.FC<ToggleVisibilityProps> = ({
+  className,
+  iconSize = "24",
+  wallet = false,
+  hideBalance,
+  setHideBalance
+}) => {
+  const toggleBalance = (e: React.MouseEvent<HTMLButtonElement>) => {
+    try {
+      e.stopPropagation();
+      setHideBalance(!hideBalance);
+    } catch (error) {
+      console.error('Failed to toggle balance visibility:', error);
+    }
+  };

   return (
-    <button className={className} onClick={toggleBalance}>
+    <button
+      className={className}
+      onClick={toggleBalance}
+      type="button"
+      aria-label={hideBalance ? "Show balance" : "Hide balance"}
+      aria-pressed={!hideBalance}
+    >

Consider renaming the component to BalanceVisibilityToggle to better reflect its purpose.

📝 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
const ToggleVisibility: React.FC< ToggleVisibilityProps> = ({ address, className, iconSize = "24",wallet=false, hideBalance, setHideBalance }) => {
const toggleBalance = (e: React.MouseEvent<HTMLButtonElement>) => {
e.stopPropagation();
setHideBalance(!hideBalance);
};
return (
<button className={className} onClick={toggleBalance}>
{!hideBalance ? (
<EyeIcon width={iconSize} color={wallet ? undefined : "#F4FAFF"} />
) : (
<EyeOffIcon width={iconSize} color={wallet ? undefined : "#F4FAFF"}/>
)}
</button>
);
};
const ToggleVisibility: React.FC<ToggleVisibilityProps> = ({
className,
iconSize = "24",
wallet = false,
hideBalance,
setHideBalance
}) => {
const toggleBalance = (e: React.MouseEvent<HTMLButtonElement>) => {
try {
e.stopPropagation();
setHideBalance(!hideBalance);
} catch (error) {
console.error('Failed to toggle balance visibility:', error);
}
};
return (
<button
className={className}
onClick={toggleBalance}
type="button"
aria-label={hideBalance ? "Show balance" : "Hide balance"}
aria-pressed={!hideBalance}
>
{!hideBalance ? (
<EyeIcon width={iconSize} color={wallet ? undefined : "#F4FAFF"} />
) : (
<EyeOffIcon width={iconSize} color={wallet ? undefined : "#F4FAFF"}/>
)}
</button>
);
};


export default ToggleVisibility;
29 changes: 29 additions & 0 deletions components/UI/iconsComponents/icons/eyeIcon.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import React, { FunctionComponent } from "react";

const EyeIcon: FunctionComponent<IconProps> = ({ width, color }) => {
Marchand-Nicolas marked this conversation as resolved.
Show resolved Hide resolved
return (
<svg
width={width}
height={width}
cursor="pointer"
Marchand-Nicolas marked this conversation as resolved.
Show resolved Hide resolved
viewBox="0 0 24 24"
fill="none"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M12 5C7 5 2.73 8.11 1 12.5C2.73 16.89 7 20 12 20C17 20 21.27 16.89 23 12.5C21.27 8.11 17 5 12 5Z"
fill={color}
/>
<path
d="M12 16.5C14.21 16.5 16 14.71 16 12.5C16 10.29 14.21 8.5 12 8.5C9.79 8.5 8 10.29 8 12.5C8 14.71 9.79 16.5 12 16.5Z"
fill={color}
/>
<path
d="M12 10.5C13.38 10.5 14.5 11.62 14.5 13C14.5 14.38 13.38 15.5 12 15.5C10.62 15.5 9.5 14.38 9.5 13C9.5 11.62 10.62 10.5 12 10.5Z"
fill="black"
/>
</svg>
Marchand-Nicolas marked this conversation as resolved.
Show resolved Hide resolved
);
};

export default EyeIcon;
45 changes: 45 additions & 0 deletions components/UI/iconsComponents/icons/eyeOffIcon.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import React, { FunctionComponent } from "react";

const EyeOffIcon: FunctionComponent<IconProps> = ({ width, color }) => {
Marchand-Nicolas marked this conversation as resolved.
Show resolved Hide resolved
return (
<svg
width={width}
height={width}
cursor="pointer"
viewBox="0 0 24 24"
fill="none"
xmlns="http://www.w3.org/2000/svg"
>
Comment on lines +5 to +12
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 removing hardcoded cursor style.

The cursor style should be controlled by the parent component for better reusability. The icon might not always be clickable in every context.

    <svg
      width={width}
      height={width}
-     cursor="pointer"
      viewBox="0 0 24 24"
      fill="none"
      xmlns="http://www.w3.org/2000/svg"
    >
📝 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
<svg
width={width}
height={width}
cursor="pointer"
viewBox="0 0 24 24"
fill="none"
xmlns="http://www.w3.org/2000/svg"
>
<svg
width={width}
height={width}
viewBox="0 0 24 24"
fill="none"
xmlns="http://www.w3.org/2000/svg"
>

<path
d="M14.12 14.12C13.8 14.46 13.39 14.75 12.92 14.92C12.55 15.08 12.15 15.15 11.75 15.11C11.35 15.08 10.96 14.95 10.61 14.72C10.26 14.5 9.96 14.18 9.73 13.8C9.5 13.4 9.37 12.96 9.34 12.5C9.32 12.1 9.39 11.7 9.55 11.33C9.71 10.96 9.95 10.62 10.27 10.34"
stroke={color}
strokeWidth="1.5"
strokeLinecap="round"
strokeLinejoin="round"
/>
<path
d="M17.94 17.94C16.26 19.36 14.13 20 12 20C7 20 2.73 16.89 1 12.5C2.06 9.93 3.92 7.86 6.29 6.51"
stroke={color}
strokeWidth="1.5"
strokeLinecap="round"
strokeLinejoin="round"
/>
<path
d="M9.9 5.06C10.67 4.9 11.45 4.84 12.24 4.89C16.24 5.11 20.27 8.24 22 12.5C21.33 14.03 20.38 15.41 19.21 16.53"
stroke={color}
strokeWidth="1.5"
strokeLinecap="round"
strokeLinejoin="round"
/>
<path
d="M3 3L21 21"
fill="black"
strokeWidth="1.5"
strokeLinecap="round"
strokeLinejoin="round"
/>
Comment on lines +34 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.

⚠️ Potential issue

Remove hardcoded fill color from path.

The last path has a hardcoded fill="black" while other paths don't have a fill color. This could interfere with the color prop's functionality and create inconsistent styling.

      <path
        d="M3 3L21 21"
-       fill="black"
+       stroke={color}
        strokeWidth="1.5"
        strokeLinecap="round"
        strokeLinejoin="round"
      />

Committable suggestion skipped: line range outside the PR's diff.

</svg>
);
};

export default EyeOffIcon;
33 changes: 20 additions & 13 deletions components/UI/profileCard/profileCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { hexToDecimal } from "@utils/feltService";
import { Url } from "next/dist/shared/lib/router/router";
import { TEXT_TYPE } from "@constants/typography";
import Typography from "../typography/typography";
import ToggleVisibility from "@components/UI/ToggleVisibility";

const ProfileCard: FunctionComponent<ProfileCard> = ({
rankingData,
Expand All @@ -37,7 +38,7 @@ const ProfileCard: FunctionComponent<ProfileCard> = ({
const sinceDate = useCreationDate(identity);
const { data: profileData } = useStarkProfile({ address: identity.owner });
const [userXp, setUserXp] = useState<number>();

const [hideBalance, setHideBalance] = useState(true);

const rankFormatter = useCallback((rank: number) => {
if (rank > 10000) return "+10k";
Expand Down Expand Up @@ -87,20 +88,26 @@ const ProfileCard: FunctionComponent<ProfileCard> = ({

<div className="flex flex-col h-full justify-center">
<Typography type={TEXT_TYPE.BODY_SMALL} color="secondary" className={styles.accountCreationDate}>
{sinceDate ? `${sinceDate}` : ""}
{sinceDate ? ${sinceDate} : ""}
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 template literal syntax errors.

There are syntax errors in the template literals that need to be corrected.

-              {sinceDate ? ${sinceDate} : ""}
+              {sinceDate ? sinceDate : ""}
-            <Typography type={TEXT_TYPE.H2} className={${styles.profile_name} mt-2}>{identity.domain.domain}</Typography>
+            <Typography type={TEXT_TYPE.H2} className={`${styles.profile_name} mt-2`}>{identity.domain.domain}</Typography>

Also applies to: 93-93

🧰 Tools
🪛 Biome

[error] 91-91: expected : but instead found {

Remove {

(parse)


[error] 91-91: expected } but instead found :

Remove :

(parse)


[error] 91-91: Unexpected token. Did you mean {'}'} or &rbrace;?

(parse)

</Typography>
<Typography type={TEXT_TYPE.H2} className={`${styles.profile_name} mt-2`}>{identity.domain.domain}</Typography>
<Typography type={TEXT_TYPE.H2} className={${styles.profile_name} mt-2}>{identity.domain.domain}</Typography>
Comment on lines +91 to +93
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 template literal syntax errors.

There are syntax errors in the template literals.

-              {sinceDate ? ${sinceDate} : ""}
+              {sinceDate ? sinceDate : ""}
-            <Typography type={TEXT_TYPE.H2} className={${styles.profile_name} mt-2}>{identity.domain.domain}</Typography>
+            <Typography type={TEXT_TYPE.H2} className={`${styles.profile_name} mt-2`}>{identity.domain.domain}</Typography>
📝 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
{sinceDate ? ${sinceDate} : ""}
</Typography>
<Typography type={TEXT_TYPE.H2} className={`${styles.profile_name} mt-2`}>{identity.domain.domain}</Typography>
<Typography type={TEXT_TYPE.H2} className={${styles.profile_name} mt-2}>{identity.domain.domain}</Typography>
{sinceDate ? sinceDate : ""}
</Typography>
<Typography type={TEXT_TYPE.H2} className={`${styles.profile_name} mt-2`}>{identity.domain.domain}</Typography>
🧰 Tools
🪛 Biome

[error] 91-91: expected : but instead found {

Remove {

(parse)


[error] 91-91: expected } but instead found :

Remove :

(parse)


[error] 91-91: Unexpected token. Did you mean {'}'} or &rbrace;?

(parse)


[error] 93-93: expected } but instead found {

Remove {

(parse)


[error] 93-93: expected ... but instead found styles

Remove styles

(parse)


[error] 93-93: Unexpected token. Did you mean {'}'} or &rbrace;?

(parse)


[error] 93-93: Unexpected token. Did you mean {'>'} or &gt;?

(parse)


{/* Balance visibility toggle section */}
<div className={styles.address_div}>
<CopyAddress
address={identity?.owner ?? ""}
iconSize="24"
className={styles.copyButton}
wallet={false}
/>
<Typography type={TEXT_TYPE.BODY_SMALL} className={styles.addressText} color="secondary">
{minifyAddress(addressOrDomain ?? identity?.owner, 8)}
</Typography>
<span className={styles.hide}>
{hideBalance ? ("*") : ("$ 2,334.34")}
</span>
Comment on lines +97 to +99
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

Add loading state for balance display.

While the balance visibility toggle is implemented, the component needs to handle the loading state while fetching the actual balance.

+  const [isBalanceLoading, setIsBalanceLoading] = useState(true);
+
   return (
     // ...
               <span className={styles.hide}>
-              {hideBalance ? ("*") : ("$ 2,334.34")}
+              {isBalanceLoading ? (
+                <span className="animate-pulse">Loading...</span>
+              ) : (
+                hideBalance ? ("*****") : ("$ 2,334.34")
+              )}
               </span>

Committable suggestion skipped: line range outside the PR's diff.

<ToggleVisibility
address={identity?.owner ?? ""}
iconSize="24"
className={styles.copyButton}
wallet={false}
hideBalance={hideBalance}
setHideBalance={setHideBalance}
/>
Comment on lines +100 to +107
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

Improve ToggleVisibility component props.

The component props could be improved for better type safety and clarity:

-              <ToggleVisibility
-                      address={identity?.owner ?? ""}
-                      iconSize="24"
-                      className={styles.copyButton}
-                      wallet={false}
-                      hideBalance={hideBalance}
-                      setHideBalance={setHideBalance}
-                    />
+              <ToggleVisibility
+                address={identity?.owner ?? ""}
+                iconSize={24}
+                className={styles.copyButton}
+                hideBalance={hideBalance}
+                onToggle={setHideBalance}
+              />

Consider:

  1. Using a number type for iconSize instead of string
  2. Removing the unused wallet prop
  3. Renaming setHideBalance to onToggle for better semantics
  4. Adding proper TypeScript prop types validation
📝 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
<ToggleVisibility
address={identity?.owner ?? ""}
iconSize="24"
className={styles.copyButton}
wallet={false}
hideBalance={hideBalance}
setHideBalance={setHideBalance}
/>
<ToggleVisibility
address={identity?.owner ?? ""}
iconSize={24}
className={styles.copyButton}
hideBalance={hideBalance}
onToggle={setHideBalance}
/>

</div>
{/* { Balance visibility toggle section close} */}

<div className="flex sm:hidden justify-center py-4">
<SocialMediaActions identity={identity} />
<Link href={shareLink} target="_blank" rel="noreferrer">
Expand Down Expand Up @@ -180,4 +187,4 @@ const ProfileCard: FunctionComponent<ProfileCard> = ({
);
};

export default ProfileCard;
export default ProfileCard;
8 changes: 7 additions & 1 deletion docs/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ To set up a development environment, please follow these steps:
cd starknet.quest
npm i && npm run dev
```
###
To avoid any failure to resolve the dependency tree, the --force flag should be added:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not the aim of this issue, please remove any change made on this file (someone else is already working on it)


```
npm i --force
```
Comment on lines +18 to +23
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 markdown formatting issues

The section has the following formatting issues:

  1. The header on line 18 is incomplete
  2. The code block is missing language specification

Apply these changes:

-### 
+### Installing dependencies
 
 To avoid any failure to resolve the dependency tree, the --force flag should be added:
 
-```
+```sh
 npm i --force

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 Markdownlint</summary>

21-21: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>

<!-- This is an auto-generated comment by CodeRabbit -->


## Issues and feature requests

Expand Down Expand Up @@ -56,4 +62,4 @@ Please try to create bug reports that are:
### Understanding the labels on your Pull Request
- If your PR is ready for review or merging then add a label which says - **Ready for Review**
- If your PR is in progress and is not ready for review or merging then add a label which says - **In progress do not merge**
- If the maintainer has reviewed and has asked for a change then they would attach the label - **Changes Requested**. You can address the comments made in the review and add the label **Ready for review** once done so that they can come back to it later and complete the review and merge 🎉
- If the maintainer has reviewed and has asked for a change then they would attach the label - **Changes Requested**. You can address the comments made in the review and add the label **Ready for review** once done so that they can come back to it later and complete the review and merge 🎉
10 changes: 10 additions & 0 deletions styles/dashboard.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@
width: 100%;
}

.hide{
font-family: Sora;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest you to use prettier to format the code correctly automatically (because the syntax is not clean in a lot of places)

font-size: 18px;
font-weight: 700;
line-height: 24px;
letter-spacing: 0.01em;
text-align:left;
transition: opacity 0.2s ease-in-out;
}

.profileCardLoading {
position: absolute;
top: 0;
Expand Down
7 changes: 7 additions & 0 deletions tailwind.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@
module.exports = {
content: ["./app/**/*.{html,tsx}", "./components/**/*.{html,tsx}"],
theme: {
extend: {
fontFamily: {
sora: ['Sora', 'sans-serif'],
},
},

colors: {
primary: "#6AFFAF",
secondary: "#F4FAFF",
Expand All @@ -20,6 +26,7 @@ module.exports = {
300: "#1F1F25",
900: "#1a202c",
},

// ... Other colors you want to add
},
},
Expand Down