interledger / web-monetization-extension

An open-source browser extension that enables Web Monetization.
Apache License 2.0
72 stars 5 forks source link

feat(popup/Settings)!: split settings into tabbed interface #657

Closed sidvishnoi closed 1 month ago

sidvishnoi commented 1 month ago

Context

Changes proposed in this pull request

github-actions[bot] commented 1 month ago

Extension builds preview

Name Link
Latest commit faef3adc7ce68f19043cab2cb0d9f2690b9b860d
Latest job logs Run #11365623457
BadgeDownload
BadgeDownload
sidvishnoi commented 1 month ago

Screenshots

Wallet screen
Wallet screen, advanced expanded
Budget screen
Rate of pay screen
Rate of pay screen, WM disabled
RabebOthmani commented 1 month ago

@sidvishnoi few comments:

sidvishnoi commented 1 month ago

For the rate of pay, I thought we were getting rid of the slider?

Eventually yes, but not in this PR. It'll get too big, and that input isn't as simple if done right.

What happens when the user changes the rate of pay or budget amount?

Rate of pay gets handled immediately, like before. Budget can't be changed at the moment. It's a big change. Those fields are disabled/readonly.

RabebOthmani commented 1 month ago

Ok for rate of pay slider. As for the budget, we can't just say something is not going to be done because it's big change. The requirement was never for it to be read-only. In the last call, we agreed that they can change the budget but it will completely override the previous grant and starts immediately. @raducristianpopa keep me honest here

sidvishnoi commented 1 month ago

We'd need UI to support edits first then. We need submit buttons somewhere, and a messaging that they'll need to connect wallet again (it'll open new tab to connect that is, so we can't just to do it in input).

RabebOthmani commented 1 month ago

Why do they need to connect to the wallet again?

RabebOthmani commented 1 month ago

Does that mean that switching between monthly or not will cause the same issues

sidvishnoi commented 1 month ago

When they change budget (any part of it - amount or recurring), we need to create a new interactive grant (i.e. ask user for permission), which happens in a new tab. So, it's the same connect wallet process again, but only without the need to add key again. (This is not an extension thing, it's how Open Payments is designed)

Does that mean that switching between monthly or not will cause the same issues

Yes. Same issues. Changing any part of budget/grant.

RabebOthmani commented 1 month ago

You mean the part on the browser to accept the grant? or the connect screen on the extension? That also mean that the monthly toggle is the wrong control. A toggle is to make changes not just for visual effects

sidvishnoi commented 1 month ago

You mean the part on the browser to accept the grant?

Yes.

That also mean that the monthly toggle is the wrong control. A toggle is to make changes not just for visual effects

Yes, wrong control if it's not meant to be editable. Point #3 in Slack

RabebOthmani commented 1 month ago

Ok. Settings should be adjustable. A user should have the ability to change the budget allocated to use with the extension and whether or not they want to renew it monthly. They will have to accept the grant online. That is absolutely fine. We will add a submit changes button that gets enabled if the amount or renew values change. Similar to connect, when they click , they get redirected to the browser.

Please do let me know if anything is not clear or if there are any technical limitations before deciding on requirements on your own. budget

sidvishnoi commented 1 month ago

Will do budget-edit support as a follow-up PR. Keeping this PR limited to tab-UI thing only.