Skip to content
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

Funding request multiple recipient preview #4509

Merged

Conversation

Copy link
Member

@thesan thesan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything else looks good 👍

Comment on lines 83 to 92
if (proposalDetails?.type === 'fundingRequest') {
return [
{
renderType: 'Amount',
label: 'Current WG Budget',
value: group?.budget,
},
] as RenderNode[]
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this works. I'm not sure why but the WG budget that this proposal should show is the content WG.

Here proposalDetails?.group?.id doesn't exist so the budget remain at 0.
Check https://dao-git-fork-vrrayz-funding-request-multiple-r-8b2aed-joystream.vercel.app/#/proposals/preview/374 (if the QN let you)
Screen Shot 2023-08-31 at 11 26 12

Comment on lines 72 to 76
result.push({
label: '',
value: undefined,
renderType: 'Divider',
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also if the page shows one more "statistic" I think this should be removed

Suggested change
result.push({
label: '',
value: undefined,
renderType: 'Divider',
})

Copy link
Member

@thesan thesan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job!

Copy link
Member

@thesan thesan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed on discord, the design changed: #2365 (comment)

Copy link
Member

@thesan thesan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@@ -99,7 +113,7 @@ export const ProposalDetails = ({ proposalDetails }: Props) => {
}

return []
}, [membershipPrice, !group])
}, [membershipPrice, !group, budget])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one !
You even fixed the Update Working Group Budget council budget preview as a bonus !
E.g https://pioneerapp.xyz/#/proposals/preview/519

@thesan thesan merged commit cb7332c into Joystream:dev Sep 6, 2023
6 checks passed
@chrlschwb chrlschwb removed the qa-task label Sep 22, 2023
@thesan thesan added the community-dev issue suitable for community-dev pipeline label Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-dev issue suitable for community-dev pipeline
Projects
Status: Tested - Ready for Prod
Development

Successfully merging this pull request may close these issues.

3 participants