-
Notifications
You must be signed in to change notification settings - Fork 130
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
Refactor/frontend#139 #142
Conversation
@phklive is attempting to deploy a commit to the Starknet Builders Team on Vercel. A member of the Team first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor changes.
Guud work ser 🔥
</Tooltip> | ||
); | ||
return ( | ||
<Tooltip title={title} arrow> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This your local prettier rules right ? if you can use the one from the repo it would be great !
import MainIcon from "../UI/iconsComponents/icons/mainIcon"; | ||
import SocialMediaActions from "./actions/socialmediaActions"; | ||
|
||
interface identityCardProps { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only use types ;).
The main difference that I know is that interface can extend other interfaces and it's not the case for types. That's the main reason why I'm using types for components
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and the first letter must be in uppercase 🔥
utils/stringService.ts
Outdated
return (firstPart + "..." + secondPart).toLowerCase(); | ||
} | ||
|
||
export function responsiveDomain( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a good naming imo cause it's not only used for responsive.
You can use breakDomain
or shortenDomain
for example.
What do you think ?
pages/identities/[tokenId].tsx
Outdated
) : ( | ||
<> | ||
<div className="flex flex-col items-center lg:flex-row w-full p-4 justify-center lg:gap-20 h-full"> | ||
<div className="w-11/12 mt-40 mb-10 lg:mt-0 lg:mb-0 lg:w-6/12 h-full flex justify-center items-center"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like when there is too much inline css with tailwind.
I understand that it can be faster than css tho, you can start with that and then use https://tailwind-to-css.vercel.app/ in order to create real css in another file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's dope 🔥
Some few things that you forgot.
1) Change the leaves here with new ones
2) Change the png leaves to svg ones (you can keep the image component but just change the URL).
3) Fix the prod thing with the font
4) Change the default font to what the designer proposed font (Poppins)
Check here how I did it this morning
5) Change all the TITLE fonts to what the designer proposed
You can search by "Montserrat Extra-Bold" and you just use quick zap instead. Did it also this morning on the previous file.
6) Refactor responsive on iPad
There is a too-big margin-top on the tablet and the card is too big when it breaks imo (because you used width in %, you just need to add a max-width to it).
…et.id into refactor/frontend#139
First draft for new identity UI.
According to the figma sent by @fricoben.
closes #139.
Some breaking changes.
Time worked 10 hours.