leather-io / extension

Leather browser extension
https://leather.io
MIT License
293 stars 140 forks source link

Use correct header for RPC screens (PSBT, arb message signing etc.) #5135

Closed fabric-8 closed 4 months ago

fabric-8 commented 5 months ago

Image Image

pete-watters commented 5 months ago

We can easily change it so everywhere just has the logo. Is that what we want? I had to reverse engineer this based on what we have in production as it’s unclear what was desired.

Having just the logo means that users are unable to switch the account they are using or see their available balance. Can we confirm that’s OK and we definitely want to only show the logo?

Having the header change from logo to back based on whether its a multistep flow is a non-trivial change as right now the headers are determined based on the route we hit. I think that work is better left to the approval UX project pending clearer clarification on exactly what flows to do it on.

fabric-8 commented 5 months ago

@pete-watters Thanks!

Having just the logo means that users are unable to switch the account they are using or see their available balance. Can we confirm that’s OK and we definitely want to only show the logo?

Yes that was the decision for now to keep follow-up work here as simple as possible and push brand recognition a bit in the meantime. If you feel rather strong about this we can also go with account + balance, we'd then just need to tweak the design a tad.

You mention account switching in the header — I thought we're only displaying account info up there without allowing the user to switch?

pete-watters commented 5 months ago

When implementing, I used the test app to help me figure out how the pop outs were working in production.

Currently the account switcher is enabled for some popups. You can see in this video with the production version to the left and the latest dev version (including global header) on the right:

https://github.com/leather-wallet/extension/assets/2938440/6b2a542f-4906-4aa2-9d41-7608bf11a5e8

I'm happy to change this as needed but I think we should make sure to think it through first and it seems like it's better to do it in the UX approval work if this is how it was already working in production.

fabric-8 commented 5 months ago

I just had another look at Michelly's earlier review where the conclusion has also been to just use the logo header for now.

I agree that it's not the ideal, however I also think we should just keep additional work at a minimum at this point while making sure things look good for the "brand update". In order to make this current account approach work, we'd have to:

...and all of this will be gone again with the approval ux redesign. So let's go with the logo approach - I guess the problem with not being able to "go back" when doing a send via the API popup has existed earlier already, so def. not something we have to address right here / right now

pete-watters commented 5 months ago

I can change this to have only the logo. Michelly's earlier review didn't include all of the pop outs and I have already made the ones she reviewed have a logo.

pete-watters commented 4 months ago

I'll close this as I think it's done or will be done in Approval