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

Update profileCard.tsx #934

Closed
wants to merge 1 commit into from
Closed
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
29 changes: 13 additions & 16 deletions components/UI/profileCard/profileCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import xpIcon from "public/icons/xpBadge.svg";
import useCreationDate from "@hooks/useCreationDate";
import shareSrc from "public/icons/share.svg";
import theme from "@styles/theme";
import EyeIcon from "../iconsComponents/icons/eyeIcon";
import {EyeIcon,EyeIconSlashed } from "../iconsComponents/icons/eyeIcon";
import ProfilIcon from "../iconsComponents/icons/profilIcon";
import Link from "next/link";
import SocialMediaActions from "../actions/socialmediaActions";
Expand Down Expand Up @@ -133,21 +133,18 @@ const ProfileCard: FunctionComponent<ProfileCardProps> = ({
>
{identity.domain?.domain || "Unknown Domain"}
</Typography>
<div className={styles.address_div}>
<div className="flex items-center gap-2">
<Typography
type={TEXT_TYPE.BODY_SMALL}
className={`${styles.wallet_amount} font-extrabold`}
>
{totalBalance !== null ? (
`$${totalBalance.toFixed(2)}`
) : (
<Skeleton variant="text" width={60} height={30} />
)}
</Typography>
<EyeIcon />
</div>
</div>
<div className={styles.address_div}>
<div className="flex items-center gap-2">
<Typography type={TEXT_TYPE.BODY_SMALL} className={`${styles.wallet_amount} font-extrabold`}>
{totalBalance !== null ? (hidePortfolio ? "******" : `$${totalBalance.toFixed(2)}`) : (
<Skeleton variant="text" width={60} height={30} />
)}
</Typography>
<div onClick={() => setHidePortfolio(!hidePortfolio)} className={styles.pointer}>
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

The pointer style needs to be added to the element.

The styles.pointer class is not defined in the imported dashboard.module.css. You need to either:

  • Add the cursor: pointer style directly to the element using inline styles
  • Or define the .pointer class in the CSS module with cursor: pointer property
  • Or use the existing custom pointer cursor style from the codebase: cursor: url(../public/icons/pointer-cursor.png), pointer
🔗 Analysis chain

Verify cursor pointer styles.

The PR objectives mention adding cursor pointer on hover, but the implementation relies on a CSS class that isn't visible in the provided code.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for the pointer class definition in CSS files
rg -t css "\.pointer\s*{[^}]*cursor:\s*pointer" 

Length of output: 180


Script:

#!/bin/bash
# Search for the pointer class definition in CSS/SCSS files with a corrected regex
rg -t css "\.pointer.*cursor:\s*pointer"

# Also search for the styles import to locate the CSS module
fd -e css -e scss -e module.css -e module.scss . | xargs rg "pointer"

Length of output: 3845


Script:

#!/bin/bash
# Let's check the specific CSS module imported in the profileCard component
rg -l "from.*\.module\.css" components/UI/profileCard/profileCard.tsx

# And then check its contents
fd -e module.css -e module.scss . | grep -i "profile" | xargs cat

Length of output: 4947


Script:

#!/bin/bash
# Let's check what styles are imported in the profileCard component
rg "import.*styles.*from" components/UI/profileCard/profileCard.tsx

Length of output: 120


Script:

#!/bin/bash
# Let's check the dashboard.module.css for the pointer class
cat styles/dashboard.module.css

Length of output: 7467

{hidePortfolio ? <EyeIconSlashed /> : <EyeIcon />}
</div>
Comment on lines +143 to +145
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 accessibility for the toggle button.

The div acting as a button lacks proper accessibility attributes.

-<div onClick={() => setHidePortfolio(!hidePortfolio)} className={styles.pointer}>
+<button
+  onClick={() => setHidePortfolio(!hidePortfolio)}
+  className={styles.pointer}
+  aria-label={hidePortfolio ? "Show balance" : "Hide balance"}
+  type="button"
+>
   {hidePortfolio ? <EyeIconSlashed /> : <EyeIcon />}
-</div>
+</button>
📝 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
<div onClick={() => setHidePortfolio(!hidePortfolio)} className={styles.pointer}>
{hidePortfolio ? <EyeIconSlashed /> : <EyeIcon />}
</div>
<button
onClick={() => setHidePortfolio(!hidePortfolio)}
className={styles.pointer}
aria-label={hidePortfolio ? "Show balance" : "Hide balance"}
type="button"
>
{hidePortfolio ? <EyeIconSlashed /> : <EyeIcon />}
</button>

</div>
</div>
<div className="flex sm:hidden justify-center py-4">
<SocialMediaActions identity={identity} />
{tweetShareLink && (
Expand Down