feat: redesign send rpc transfer flow

[alter-eggo]

Try out Leather build 699bf7d — Extension build, Test report, Storybook, Chromatic

redesign rpc send transfer using approver ux

[kyranjamie]

Great work @alter-eggo, leaving some comments.

I'll merge my PR now, which has some of the changes to focusTab helper you can use here

[kyranjamie]

Shouldn't these presentation/formatting concerns be directly in component?

[alter-eggo]

we will use it in other bitcoin flows as well, so prefer keep in this file, though took it out of fees hook

[pete-watters]

Maybe you can use a .layout file to help split this up between logic and presentation?

[alter-eggo]

don't understand how to split tbh and why need? what should be in .layout?

[kyranjamie]

There are components with many style props, can't they be in a layout component?

In getAddresses the entire Approver component is in the layout, such that the root RPC compoent can be focused on business logic/event passing behaviours, and not layout and styles.

[alter-eggo]

looking at this file I believe main problem here is with huge approve logic. I will try to move this to sep file.
actually we'll need this in other bitcoin related flows as well

[alter-eggo]

same with selected fee

[alter-eggo]

and prob then will be way easier to move it to layout file. will try

[pete-watters]

Great work here @alter-eggo 🚀 huge PR!

I added a few suggestions but looking good to me overall

[fbwoolf]

Nice work @alter-eggo! I migrated some of the bitcoin fees code to the mono repo for mobile. I have an issue open to install it back into the extension to make sure it is shared, but that can come later.

[kyranjamie]

Building on earlier comment, why don't we do the capitalize in the view too? How we format/present data is a concern of the UI so better lives where it's set to a component.

My thinking is that if I as a dev need to go and make visual changes, they can be mostly scoped to a set of .layout components. But in this file, we have a mix of mathematics and formatting, that I can't help but feel should live separately.

e.g. <style.span>{capitalize(type)}</styled.span>

Then, the utils file knows nothing about where (titleLeft), or how (capitalised), the data needs to be displayed, focusing on accurate data only.

What's your thinking for putting it here, and are there reasons I'm missing why it's beneficial to group it here?

[alter-eggo]

I tried to do FeeItem as dumb as possible, so general idea here was to gen and pass ready to use parts of the fee component, title/caption left/right

yeah, maybe better to capitalize in component itself, and so remove titleLeft, I just thought it would be better to keep all these parts in one place.

so in stx I can do the same - get raw fee => then gen ready to use parts of fee component

Then, the utils file knows nothing about where (titleLeft), or how (capitalised), the data needs to be displayed, focusing on accurate data only.

if not utils so what's the best place to keep it? other way btw - create bitcoin fee wrapper for shared fee item and keep this logic there

[markmhendrickson]

Looking good! Some QA feedback:

• App favicon isn't showing up in the header
• "Total spend" doesn't include the fee amount
• Guide URL takes user to https://leather.io/guides/connect-dapps instead of a relevant guide for sending BTC via an app
• Do we want to say "Send bitcoin" instead of "Send BTC"? cc @fabric-8 @mica000
• I'd suggest some additional design QA (by a designer) to double check text styling / sizes, padding, etc.
• Header for edit fee screen has unexpected top and bottom padding in beige

[fabric-8]

Looking great, only some minor feedback:

• animal icon stroke width are too thin, should generally be same as for the other icons. Maybe they are getting scaled because there's a mismatch of height/width it needs (32x32) vs. actual size after some... padding is applied? If I can help by adjusting the asset lmk, the animation icons are 32x32 vs. 24x24, if turning them into 24x24 fixes this issue I can do it

• Since were are using the > to indicate that the user can interact with the item (like for account on top), I think we should also add the > to fee settings for consistency cc @mica000

• Realising that "cancel" in fee settings might get misinterpreted to "cancel the whole approval thing" - Maybe using the word "back" would be better here? cc @markmhendrickson

• Let's add border-top: 1px solid ink.border-default to the footer