Skip to content

Conversation

ekzyis
Copy link
Member

@ekzyis ekzyis commented Jul 30, 2025

Description

based on #2397 close #2308

This PR introduces a three-step flow to attach wallets: 1. attach send 2. attach receive 3. settings

The settings can only be accessed if the wallet has at least one send or receive configuration (else we wouldn't be saving anything).

/wallets/settings has been removed because the settings can now be accessed by going through the flow.

After a wallet has been configured, each protocol can still be enabled/disabled via the familiar checkbox in the protocol form. Individual protocols can no longer be detached however; a wallet can only be fully deleted via the 'delete' button in the settings (not quite happy with the position of it though).

The logic for the multi-step form has been encapsulated into @/components/multi-step-form.js. It's agnostic of wallets and handles:

  • rendering the progress and children based on which step we're currently on
  • keeping form state across steps
  • navigation, including via browser
TODO
  • ✅ skip if form empty, else show 'next' and test credentials
  • ✅ allow browser navigation to go back, too
  • ✅ move wallet tests into own mutation
  • ✅ save wallets
  • ✅ update wallets
  • ✅ handle wallets without send or receive
  • ✅ fix detach vs cancel
  • ✅ populate fields like url from previous form
  • ✅ q&a
  • ✅ refactor multi-form code into own component
  • ✅ make enable/disable send/recv vs delete more clear

Video

2025-08-09.17-07-10.mp4

Additional Context

  1. As mentioned in the description, I'm not quite happy with the position of the 'delete' button but not sure where else to put it.

  2. This bullet point from Wallet flow #2308 has been skipped because it's better suited for a follow-up PR:

  • at the end, dedicated page for gun and horse so they know what it means when they see it next to the nym of other stackers

Checklist

Are your changes backward compatible? Please answer below:

yes

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:

8. Tested attaching, configuring and deleting wallets that use lightning addresses, NWC or LNbits

For frontend changes: Tested on mobile, light and dark mode? Please answer below:

n/a

Did you introduce any new environment variables? If so, call them out explicitly here:

no

@ekzyis ekzyis added feature new product features that weren't there before wallets labels Jul 30, 2025
@ekzyis ekzyis marked this pull request as draft July 30, 2025 00:06
@ekzyis ekzyis force-pushed the wallet-flow branch 6 times, most recently from 9ca3b23 to e1c49e8 Compare August 2, 2025 21:52
@ekzyis ekzyis force-pushed the wallet-flow branch 18 times, most recently from e9ff174 to 57a7604 Compare August 8, 2025 17:15
@huumn
Copy link
Member

huumn commented Aug 12, 2025

As discussed I'm mostly suspicious of the UI. I'm going to list off things that feel off to me. If you'd like me to intervene and fix them myself, let me know.

Screenshot 2025-08-12 at 4 44 09 PM

The progress thing works well but looks sloppy. Things don't line up and aren't centered. You might consider using svgs of circled numbers.

Screenshot 2025-08-12 at 4 45 41 PM

I find the form back button confusing as it matches the page back button. imo it should be < back and the text of the next should be next >. These will also help distinguish the look of the form nav from cancel/skip.

Screenshot 2025-08-12 at 4 45 50 PM Screenshot 2025-08-12 at 4 46 00 PM

The tabbing for wallets deviates from the tabbing used on all other pages.


I'll continue light QA and comment with any other issues. Once that passes, I'll skim the code.

@ekzyis
Copy link
Member Author

ekzyis commented Aug 13, 2025

The progress thing works well but looks sloppy. Things don't line up and aren't centered.

Oh, I lost something during my refactor here:

steps.map((label, i) => {
const last = i === steps.length - 1
return (
<Fragment key={i}>
<ProgressNumber number={i + 1} label={label} active={stepIndex >= i} />
{!last && <ProgressLine style={{ marginLeft: '-4px', marginRight: '-13px' }} active={stepIndex >= i + 1} />}
</Fragment>
)
})

This was supposed to have a different margin adjustment for the second line. This and filling the numbers with the background color (so the line can't appear inside the circle) is how I made the lines touch the number.

You might consider using svgs of circled numbers.

Yes, will use svgs instead

I find the form back button confusing as it matches the page back button. imo it should be < back and the text of the next should be next >. These will also help distinguish the look of the form nav from cancel/skip.

Yes, good idea

The tabbing for wallets deviates from the tabbing used on all other pages.

Ah, I see that the font weight and color is off. Will make sure to reuse the CSS from nav.module.css

cursor[bot]

This comment was marked as outdated.

@ekzyis ekzyis closed this Aug 14, 2025
@ekzyis ekzyis deleted the wallet-flow branch August 14, 2025 17:29
@ekzyis ekzyis restored the wallet-flow branch August 14, 2025 17:30
@ekzyis ekzyis reopened this Aug 14, 2025
@ekzyis
Copy link
Member Author

ekzyis commented Aug 14, 2025

whoops, accidentally deleted the branch for a moment

The progress numbers now use SVGs and look like this:

localhost_3000_wallets_1_step=send(iPhone SE)

@ekzyis
Copy link
Member Author

ekzyis commented Aug 14, 2025

Now using arrow-left-s-fill etc. for the form navigation instead of the existing arrows for page navigation:

localhost_3000_wallets_1_step=receive(iPhone SE)

Had to manually adjust the padding of the next button to make it look good.

Using the theme class for the hover style of the arrow to go back was removed as now the style should apply when we hover over the parent. How hover styles are applied to SVGs needs to be rethought. Fixed in d63f201.

@huumn
Copy link
Member

huumn commented Aug 15, 2025

Looks much better. Thank you! I'm going to give it a quick QA/line-by-line and merge if all goes well.

Copy link
Member

@huumn huumn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I centered things a little more, and gave a few places padding, but it is otherwise good to go. It looks nice and works better!

I'll merge when you're more available.

@huumn huumn merged commit e46f4f0 into master Aug 26, 2025
7 checks passed
@huumn huumn deleted the wallet-flow branch August 26, 2025 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature new product features that weren't there before wallets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wallet flow
2 participants