feat: improving profile section #247
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
components/UI/menus/popup.tsx
Outdated
| buttonName, | ||
| onClick, | ||
| children, | ||
| bottomContent = null, | ||
| onClose, | ||
| }) => { | ||
| return ( | ||
| <div className={styles.popupContainer}> |
There was a problem hiding this comment.
You need to use the modal from Material UI as in wallets for example.
Because with your current custom modal some things doesn't work (like if I click outside of the modal it doesn't close), sometimes the navbar goes on top of it ...
Considering that we're using the Material Ui modal everywhere on app.starknet.id but also here, I think you should use it.
pages/[addressOrDomain].tsx
Outdated
| {typeof addressOrDomain === "string" && | ||
| isHexString(addressOrDomain) | ||
| ? minifyAddressWithChars(addressOrDomain, 8) | ||
| : minifyAddressWithChars(identity?.addr, 8)} |
There was a problem hiding this comment.
What's the goal of that ? We can make a function ? Move it to utils stringService and test it ?
There was a problem hiding this comment.
It checks if addressOrDomain is an address, if so it minifies it. Else, it gets the address from the identity and minifies it. But I don't know why we don't simply directly minify the address from the identity
fricoben
added
❌ Change request
Marchand-Nicolas
added
🔥 Ready for review
There was a problem hiding this comment.
Almost there, You have some code changes and on the front we have the following.
Make the social smaller, less gap between each icon and more space between the icon part and the share button (like in the figma).
Add more space to the achievement section (the goal of this refactoring was to add this space)
| @@ -89,3 +89,16 @@ export function numberToString(element: number | undefined): string { | |||
| export function convertNumberToFixedLengthString(number: string): string { | |||
| return number.padStart(12, "0"); | |||
| } | |||
|
|
|||
| export function minifyAddressFromStrings( | |||
There was a problem hiding this comment.
You need to test this function on stringService.test.js
Here is a way to do it using chat gpt.
https://app.skiff.com/file/8282cddd-9cd9-420c-a636-f2d81aec00c3
| @@ -235,59 +239,74 @@ const AddressOrDomain: NextPage = () => { | |||
| title={identity?.domain ?? minifyAddress(identity.addr)} | |||
| isUppercase={true} | |||
| content={ | |||
| <div className={styles.nameCard}> | |||
There was a problem hiding this comment.
Why don't we put all the content in the ProfileCard component directly? Will we use this profile elsewhere? if not I think we can put it directly in because it's cleaner imo
fricoben
added
❌ Change request
Marchand-Nicolas
added
🔥 Ready for review
close: #165 close: #225