-
Notifications
You must be signed in to change notification settings - Fork 139
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/migrate-stake-and-withdraw-page-to-tailwind #544
Conversation
@chiscookeke11 who is author of this PR? |
It's mine,
I'm not home right now so I had to finish up with my friend's repo so that I can meet up with the ETA
Sent from Outlook for Android<https://aka.ms/AAb9ysg>
…________________________________
From: djeck1432 ***@***.***>
Sent: Thursday, January 30, 2025 4:38:46 PM
To: djeck1432/spotnet ***@***.***>
Cc: Okeke Chinedu Emmanuel ***@***.***>; Mention ***@***.***>
Subject: Re: [djeck1432/spotnet] feat/migrate-stake-and-withdraw-page-to-tailwind (PR #544)
Screenshot.2025-01-30.at.16.38.08.png (view on web)<https://github.com/user-attachments/assets/3145d2e0-a7bc-4a99-a877-3623a56abd80> @chiscookeke11<https://github.com/chiscookeke11> who is author of this PR?
—
Reply to this email directly, view it on GitHub<#544 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BAYCM7AVTHXN4U4IU4EBD6L2NJBQNAVCNFSM6AAAAABWFQTL6WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMRUHA2DAMZUGM>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
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.
don't make changes to Button as originally mentioned in the issue
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.
don't make changes to Button as originally mentioned in the issue
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.
You didn't remove old css classes
<div className="mt-1.5"> | ||
<p className="text-(--primary) text-center text-lg mt-3 !-mb-5">Stake Withdrawal</p> | ||
<div className="border !border-[#36294e] p-5 rounded-lg mt-5"> | ||
<div className="flex items-center !justify-between bg-(--footer-divider-bg) border !border-[#36294e] py-10 px-5 rounded-lg w-[600px]"> |
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.
please do not use custom colors like that bg-(--footer-divider-bg)
here and in all other components
<div className="mt-1.5"> | ||
<p className="text-(--primary) text-center text-lg mt-3 !-mb-5">Stake Withdrawal</p> | ||
<div className="border !border-[#36294e] p-5 rounded-lg mt-5"> | ||
<div className="flex items-center !justify-between bg-(--footer-divider-bg) border !border-[#36294e] py-10 px-5 rounded-lg w-[600px]"> |
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.
avoid important (!) and direct colors in components, move it to theme and use correctly, check tailwind docs
Okay
On it
Sent from Outlook for Android<https://aka.ms/AAb9ysg>
…________________________________
From: whateverfw ***@***.***>
Sent: Thursday, January 30, 2025 9:27:57 PM
To: djeck1432/spotnet ***@***.***>
Cc: Okeke Chinedu Emmanuel ***@***.***>; Mention ***@***.***>
Subject: Re: [djeck1432/spotnet] feat/migrate-stake-and-withdraw-page-to-tailwind (PR #544)
@whateverfw requested changes on this pull request.
________________________________
On frontend/src/pages/stake/Stake.jsx<#544 (comment)>:
This page does not match previous versions, cards and dropdown styles are different now
(I'm aware of the fact that this page wasn't exactly like in Figma, but at least make it look the same as previous version, e.g. Withdraw page is fine)
—
Reply to this email directly, view it on GitHub<#544 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BAYCM7DE4ERDHMYU4AVIRUT2NKDM3AVCNFSM6AAAAABWFQTL6WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKOBUHE2TINRUHE>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
I've implemented all the latest changes. I have a problem with the metric card in the stake.jsx page. |
Fixed the height problem in the metric card. |
Fixed the border radius problem; a different file was being imported |
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.
@chiscookeke11 please resolve merge conflicts, apart from that everything is fine
Conflicts Fixed |
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.
lgtm
nice
…On Fri, Jan 31, 2025 at 8:29 AM whateverfw ***@***.***> wrote:
***@***.**** approved this pull request.
lgtm
—
Reply to this email directly, view it on GitHub
<#544 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BAYCM7F5G25GY6XXFHHDXKT2NPFJHAVCNFSM6AAAAABWFQTL6WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKOBXGYZTENZTHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
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.
Looks good to me
No description provided.