extension icon indicating copy to clipboard operation
extension copied to clipboard

feat: approver pattern

Open kyranjamie opened this issue 11 months ago • 3 comments

Building Leather at commit 90b1724

This PR adds the <Approver /> component as described in Notion.

Initial demo

https://github.com/leather-wallet/extension/assets/1618764/d550dbc6-c059-421a-bb7f-1f501dce807c

Todo

  • [ ] Add tooltip to Header component
  • [ ] Figure out best way to handle success screens (maybe just a duplicate Approver)
  • [ ] Add friction to Approve button cc/ @fabric-8
  • [ ] Blocked: header is outside the container currently when window's resized, not sure if this is easy to fix with Pete's work?

kyranjamie avatar Mar 05 '24 18:03 kyranjamie

The Header issue you mentioned is resolved in my work so you should be OK 👍

pete-watters avatar Mar 06 '24 05:03 pete-watters

This is amazing!!!! 😍😍😍😍😍😍

I noticed we are using 12 + 24px in the margins and 12px between ItemLayout and heading: Screenshot 2024-03-07 at 12 46 45

This is happening because we don't have the heading component in development, so the structure will naturally be different. Wondering what we could do to preserve the same values. Maybe adding 24px between heading and itemLayout and 24px around all the margin will solve it?

Image of how it is in designs: image

Also, the heading has a placeholder icon to show a tooltip that is sometimes present. Curious on how we would handle that as well. image https://www.figma.com/file/2MLHeIeL6XPVi3Tc2DfFCr/%E2%9D%96-Leather-%E2%80%93-Design-System?type=design&node-id=8945-112587&mode=design&t=pKAQcmHCAYgjWfee-4

mica000 avatar Mar 07 '24 17:03 mica000

Thanks Michelly.

I haven't made sure the margins are accurate yet, so good you point it out. think it might be best to put a min-height on the header element. Will come up with a god approach today and show you.

The margins should look visually identical to designs, but be aware of the changes made in these PRs https://github.com/leather-wallet/extension/pull/5014 https://github.com/leather-wallet/extension/pull/5024.

In Figma you've padded the ItemInteractive for the hover state, but if we build it like that then it gets complicated because we need to use "compound margins" on the container (12 + 12) and every other element to get the right spacing. See storybook for demo of this. Now we use pesudo elements for the background so the core element needs to adjustment and aligns by default.

kyranjamie avatar Mar 08 '24 09:03 kyranjamie

This PR has been extracted to the monorepo

kyranjamie avatar Sep 24 '24 09:09 kyranjamie