-
Notifications
You must be signed in to change notification settings - Fork 43
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 status transfer 2 #136
Feat/add status transfer 2 #136
Conversation
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.
Functionally, it works, but there are a few code quality issues that should be fixed before merging.
I fixed the data.length > 0 bug, but its existence makes me ask: was this tested? Do you have a wallet, some KONOHA tokens, guardianship, did you try to cancel a transfer?
I'm not thrilled about the status not being a table but instead a custom third thing that looks like a table, and yet it looks different from the Your Stakes table. But I guess we'll have to live with it.
return ( | ||
<div key={index} className="flex flex-wrap flex-row"> | ||
<div className='flex flex-wrap flex-row w-full'> | ||
<div className="flex-1 w-1/2 basis-1/2 p-2 bg-slate-200 text-left text-gray-700 uppercase tracking-wider border-r border-slate-400">token</div> |
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.
What is the reasoning behind the border-r here?
|
||
|
||
useEffect(() => { | ||
refetch() |
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.
What is the purpose of this chunk of code?
No description provided.