Use same components between proposal builder and sablier stream proposal builder #2646
Conversation
…em within ProposalBuilder
mudrila
requested review from
adamgall,
johnqh,
DarksightKellar and
Da-Colon
✅ Deploy Preview for decent-interface-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Thanks for the additional context @mudrila!
The UI on /proposals/new/sablier/metadata doesn't look identical to /proposals/new/metadata, there are a handful of differences. Maybe some or all of these are expected?
| difference | /proposals/new/metadata |
/proposal/new/sablier/metadata |
|---|---|---|
| Page title | "Create Proposal" | "Create Proposal Template" |
| Title subtext | "A short title for this proposal" | "A short title for this proposal template" |
| Description subtext | "Add a brief description, markdown supported" | "Markdown syntax supported" |
| Preview card/section | Includes general proposal information | Includes Sablier-specific information |
Nitpick on Naming
Not a change that was introduced in this PR, but I think calling the Sablier page "Create Proposal Template" isn't correct.
We're not creating a template here, we're using an existing template to create a proposal.
So maybe rename the new Sablier proposal workflow/page just "Create Proposal.
This would change the full breadcrumb from:
- "Your DAO / Proposal Templates / Create Proposal Template", to
- "Your DAO / Proposal Templates / Create Proposal"
which IMO is in line with the breadcrumb and page title for the original Create Proposal flow (at /proposals/new/metadata): "Your DAO / All Proposals / Create Proposal".
| export type ProposalTemplate = { | ||
| transactions: CreateProposalTransaction[]; | ||
| } & CreateProposalMetadata; |
There was a problem hiding this comment.
probably too much to ask here. But would love If we didn't need that union type in the form for Sablier
There was a problem hiding this comment.
FWIW this isn't a union type, it's an intersection type
https://www.typescriptlang.org/docs/handbook/2/objects.html#intersection-types
There was a problem hiding this comment.
Yeah I wanted to get a better type safety but that leads to more usually-not-necessary conditional checks and type casting. I don't mind adding that(and actually would love to) though thought it will be better to do that in separate PR - this one is already pretty big 😅
But whatever works for me - just let me know your preference @adamgall @Da-Colon
There was a problem hiding this comment.
Sorry @adamgall I was refering to the union type this is used.
There was a problem hiding this comment.
Yeah @mudrila totally can be another PR.
Oh dang - that definitely just a lack of proper conditional rendering - it is supposed to reflect page title, title subtext, description subtext from the "Create Proposal" UI. But yeah - the summary and streams creation are supposed to be slightly different (same components - different content). I'll fix this |
Changes
This PR eliminates "Stream Sablier Builder" set of custom components and leverages existing
ProposalBuilderin order to streamline and standardize UI / UX.Since "Stream Sablier Builder" was initially build as "copy-paste" adaptation of
ProposalBuildereventually UX diverged over time.Addressed issues
After merging #2647 into this PR - it will also solve following issues / requests:
[ENG-65]https://linear.app/decent-labs/issue/ENG-65/implement-airdrop-action-during-proposal-creation[ENG-66]https://linear.app/decent-labs/issue/ENG-66/add-example-proposal-templatesTesting
/proposals/new/sablier/metadata)[ENG-65][ENG-66]Create example templates for Transfer, Airdrop and Sablier stream creation #2647 - following testing steps also gonna be relevant: