-
-
Notifications
You must be signed in to change notification settings - Fork 260
feat: implement delegation Tab #268
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
Conversation
@Dozie2001 is attempting to deploy a commit to the Kalis Software 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.
Thanks @Dozie2001, this looks great overall. I left some comments here and there.
Additionally, it looks like the field for delegate and delegator (in incoming/outgoing) is not correct (just set to the current user, instead of the counterparty). We could also consider merging the incoming/outgoing delegations into a single table if that makes sense. But either way is fine.
It also looks like the revoking logic in general still needs work. When I tried to revoke a delegation, I first ran into this issue:
I pushed a quick fix for that to this PR. But still then I am running into issues revoking:
The delegation in question is a "ALL" type delegation on Delegate v2.
components/delegations/InfoPanel.tsx
Outdated
<div className="flex flex-col gap-4"> | ||
<h2 className="text-lg font-bold">{t('address.delegations.info_panel.title')}</h2> | ||
<p>{t('address.delegations.info_panel.description')}</p> | ||
<div className="flex flex-col gap-2"> | ||
<h3 className="text-md font-semibold">{t('address.delegations.info_panel.what_are_delegations')}</h3> | ||
<ul className="list-disc list-inside"> | ||
<li>{t('address.delegations.info_panel.delegations_explanation_1')}</li> | ||
<li>{t('address.delegations.info_panel.delegations_explanation_2')}</li> | ||
<li>{t('address.delegations.info_panel.delegations_explanation_3')}</li> | ||
</ul> | ||
</div> | ||
<div className="flex flex-col gap-2"> | ||
<h3 className="text-md font-semibold">{t('address.delegations.info_panel.why_revoke')}</h3> | ||
<ul className="list-disc list-inside"> | ||
<li>{t('address.delegations.info_panel.revoke_reason_1')}</li> | ||
<li>{t('address.delegations.info_panel.revoke_reason_2')}</li> | ||
<li>{t('address.delegations.info_panel.revoke_reason_3')}</li> | ||
</ul> | ||
</div> | ||
</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.
I think we need to change the content here a bit. But I will think about what that content should be.
… into delegate-dashboard
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.
The revoking functionality looks to be working now, very nice!
I left some additional comments focused on (visual) consistency and some code changes. But looks like we should be almost good to go.
…iltering (for less maintenance)
- Add potential risk factors to the delegator - Simplify incoming/outgoing columns generation & update column order - Simplify "no delegations found" - Fix <td> nesting - Retrieve potential token metadata for "contract" / "erc20/721/1155" delegations - Simplify DelegationTypeCell - Add colors to Delegation platforms - Move "TOKEN" delegation type to "ERC721" since they're very similar - Small other fixes / changes
Closes #188