juice-interface
juice-interface copied to clipboard
jbx-protocol
Fix for V2 edit payouts - amount missing when distribution is fixed
What does this PR do and why?
Link to an issue - https://github.com/jbx-protocol/juice-interface/issues/1620
The intention of this PR is to show amount input field in edit payouts for v2 projects when there is distribution limit.
Please be careful while reviewing since I've deleted some code that I thought is not necessary. I may be right or wrong but is my second PR in this repository and much more serious than the previous one :)
PREVIOUS DISCUSSION - https://github.com/jbx-protocol/juice-interface/pull/1671
Screenshots or screen recordings
https://user-images.githubusercontent.com/18723426/183590314-94a8edbb-a351-4982-b9fa-e06c7ddfcf1d.mov
Acceptance checklist
- [x] I have evaluated the Approval Guidelines for this PR.
- [x] I have tested this PR in all supported browsers.
- [x] I have tested this PR in dark mode and light mode (if applicable).
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Updated |
|---|---|---|---|
| juice-interface-rinkeby | ✅ Ready (Inspect) | Visit Preview | Aug 31, 2022 at 1:21PM (UTC) |
1 Ignored Deployment
| Name | Status | Preview | Updated |
|---|---|---|---|
| juice-interface | ⬜️ Ignored (Inspect) | Aug 31, 2022 at 1:21PM (UTC) |
![vercel[bot] avatar](https://avatars.githubusercontent.com/in/8329?v=4 "Published by a pub.dev verified publisher"){kind=small}
I'll paste your quote from closed PR here @wraeth-eth
Another problem I just noticed is that when you have amount set, you cannot set anything higher than the amount
https://user-images.githubusercontent.com/18723426/183849144-deb642e3-97d7-4791-b180-6f2203f7aa4c.mov
I apologize if i don't understand the entire flow, but is this not natural behaviour? Since distribution limit amount is set to 200 and if user tries to set anything higher than that, percentage input should warn user?
EDIT: forgive my stupidness here, this video was from reconfiguring funding, and didn't know it back then.
@wraeth-eth I've found a much easier workaround for all of this which I've tested on every scenario, I'm gonna push it soon.
Hey @NakedFool sorry i've been away for the last couple of days. Let me know when this is ready for a re-review :)
Hey @NakedFool sorry i've been away for the last couple of days. Let me know when this is ready for a re-review :)
No worries! It took me time to understand better everything and therefore I paused pushing until we both are clear here. So after some thinking, I think you had a point right when we were discussing on discord.
It makes sense to me to combine amount and percentage when the user is editing payouts because the percent is based on the distribution limit that has already been set.
Although, when the user is adding payouts as you said on discord it can become uncomfortable. Should we apply amount + percentage only when the user is editing payouts?
One note btw, when I picked this task I thought it would just be a quick fix in code, therefore I am sorry if this took too much time.
Hey @NakedFool - I think we probably need an overhaul of the entire system. It is likely we are employing the wrong solution for the problem here which would explain why it is so confusing 😛 .
Is it possible to solve the og bug without changing the ux too much here? I think we can look at overhauling this at a later date - rn the focus we have is on the project creation flow; I am sure that this will be re-configured in time 😉
Hey @NakedFool - I think we probably need an overhaul of the entire system. It is likely we are employing the wrong solution for the problem here which would explain why it is so confusing 😛 .
Is it possible to solve the og bug without changing the ux too much here? I think we can look at overhauling this at a later date - rn the focus we have is on the project creation flow; I am sure that this will be re-configured in time 😉
I think there is a way to solve og bugs without changing UX stuff, I'm working on this today!
yoo @wraeth-eth !
I've pushed commits that are responsible for og bugs, I have a few notes that I would love to let you know.
v1 vs v2
I'll compare functionalities in v1 and v2 - this stuff you already know but its good to write this so we can be connected.
In v1, in the funding reconfiguring section, we first set the distribution limit, and then we can add/edit splits with percentage or amount input fields.
In v2, in the funding reconfiguring section, we allow users to set distribution limit while adding payouts, therefore if we use a mix of the amount and percentage it can be a UX nightmare ( you had a really good comment in discord, I would love to hear thoughts of other folks when time allows )
If we decide to go with a mix, I'll be keen to take over it and implement the solution.
Minor stuff
When a user is now editing splits, we allow a mix of the amount and percentage inputs when there is a distribution limit, therefore text info needs to be changed, I'll post a screenshot so it can be clear to you automatically.
Possibly rename it to Reconfigure payouts as percentages or a fixed sum of your distribution limit. when the distribution is not infinite ?
Screen recording
https://user-images.githubusercontent.com/18723426/185640431-96baa7c6-90fc-458f-ab1f-5970cf4e7aa8.mov
@johnnyd-eth yep - it looks good to me, there is some conflicting changes though @NakedFool , we'll want to get them fixed up then I think we can probably merge
Pushed the latest changes, and things should be good now!
Discussion - https://discord.com/channels/939317843059679252/1011161626747088956/1013710657046200340
Another merge conflict here @NakedFool, our bad. @wraeth-eth or I should've merged this quicker 🙏
Closing this - the intention is there and good, but I think adding this feature will cause more problems than it will solve for now. Ultimately, we will re-add this in a rewrite of this modal