release: nostalgic skunk

[pete-watters]

Try out Leather build 051b1b5 — Extension build, Test report, Storybook, Chromatic

This release contains:

• https://github.com/leather-io/issues/issues/215
• Design System - Add containers #4370

Along with:

• feat(analytics): background analytics event
• chore: force resolution of fast-xml-parser
• chore: update react-virtuoso package
• fix: mainnet faker contract address
• Fix delete icon alignment for network #5124
• Fix hover glitch for copy option in PSBT approval modal #4972

@alter-eggo has more work to add to this but I'm opening this early as there have been some big code refactors done so it's worth a detailed pre-release test by @markmhendrickson + @314159265359879

[314159265359879]

for the next couple of days I will run this version to check it out. So far so good.

[314159265359879]

Leather logo in extension popup is gone.

Looks alright in tab (maximized)

[markmhendrickson]

Let's include #5690 as well for holistic testing and release? cc @alter-eggo

[markmhendrickson]

@314159265359879 @DeeList can you two review various API-based flows as well to verify that headers show up as expected?

[314159265359879]

I am not sure what this is about:

Tested sending assets on stacks and bitcoin. minting and listing, looks good.
I see the Leather title showing normally now.

[314159265359879]

I am not sure if the screenshot I added (that pops up when the wallet is being set up) requires immediate attention. It didn't block me on anything.

[markmhendrickson]

See issue with sending BRC-20 tokens from @DeeList here

[314159265359879]

Deelist mentioned that BRC20 looked broken. But I do not see that issue he described. Going through the BRC20 send flow, I can transmit the transaction to create the transfer inscription. There is a small issue with the title here (visual) on the confirm page:

I also tested the send flow for the transfer inscription. Looking good too.

I tested this build: release/nostalgic-skunk#ac21ff8

I do notice that sending the payment for the BRC transfer through the ordinalsbot integration shows first but it doesn't show the rest of the steps after the payment has been completed. An hour after confirmation no transfer inscription has been made. Have their been any updates to this service? (this is shown, nothing after that:)

[markmhendrickson]

The app menu now appears during the wallet creation and restoration flows (which is a good enhancement) though it also contains both "Switch account" and "Sign out" options, which should be hidden if the user hasn't yet configured / added a wallet.

Trying these options anyway results in these two error view variants:

We should fix these error views (in addition to preventing their access, by removing the two options from the menu). The latter variant should be consistently used, and its styles should be fixed (e.g. it shouldn't be left-aligned and squished up against the viewport like that without app nav up top).

In sum, these are the needs:

1. Hide "Switch account" and "Sign out" options from menu when no wallet configured
2. Create integration test per related acceptance criterion
3. Fix "Error" view styles
4. Create visual regression test for above item
5. Ensure error view is applied when triggering error from "View Secret Key" view
6. Create integration test per related acceptance criterion

[markmhendrickson]

I found this issue with the dismissal option for the hardware wallet connection view while testing this build, but it's also present in the production build. I'm flagging here in any case, in case its resolution was intended by the related containers work. If not, we can resolve separately from this PR.

[markmhendrickson]

Activity is loading quickly for me in this build. Let's also add an integration test per this acceptance criterion to prevent a regression? cc @alter-eggo

[markmhendrickson]

The logo shouldn't show up the "Add network" view when displayed in the action popup. Let's fix and add a test per this acceptance criterion.

In the title, "Add network" shouldn't capitalize "Network" either.

Note in production, it's displayed in full page view but not action popup, as desired.

Update: It appears this is a general issue across views loaded in the action popup. Anytime we have a back button, we should hide the logo?

[markmhendrickson]

Now that we've switched to Mempool.space, presumably the prefilled value for Bitcoin APIs when adding networks should also be Mempool?

[markmhendrickson]

Network menu styles look good 👌

[markmhendrickson]

The receive view now has a dismissal option rather than a back option in the app nav. @fabric-8 @mica000 I presume the back option is desired for consistency with the other home options (e.g. send, swap, buy).

Note as well that the receive header has no top margin either as a result.

In production, the back button appears:

Let's add tests for these acceptance criteria as well:

• Select token
• Receive assets
• Swap assets

[markmhendrickson]

The app navs during the various API flows look good to me.

Ideally we'd show the STX balance for the Stacks message signing variant, as we show BTC for Bitcoin message signing, but this enhancement isn't a blocker.

Bitcoin message signing

Bitcoin transaction

PSBT

Stacks message signing

Stacks transaction

[markmhendrickson]

The latest build looks generally good in terms of the menu and app nav fixes. However, note that the menu for the state with no wallet configured has a remnant divider line which should ideally be removed as well:

Note how it doesn't appear here when signed in:

I overlooked flagging #5726 (comment) yesterday as a blocker, but we'll need to resolve it before releasing this PR as well.

[fabric-8]

@mica000 Regarding back button vs. close, I agree that in the context of the popover it doesn't quite make sense from a user perspective why there would be a close for receive, but a back for the other screens.

Maybe we can just keep the "back" in the popover, but do use a close in fullscreen as already mocked up here earlier, as it's effectively a different kind of ux there? I'm not 100% sure wether that has been our intention all the ay or not since the version with "x" does exist on the same page as well

[markmhendrickson]

If needed to unblock this release today, we can also just add top padding to the receive view in the action popup format?

The lack of consistent top padding there compared to other similar views (e.g. send, swap, buy) is what seems most in need of resolution. We can figure out later if we need to convert the receive view into a page (since it's basically a sheet at the moment, whereas these other views are pages?).

Let's take care to QA PRs on the original PRs themselves going forward. We're unfortunately trying to fix these container related issues now to unblock the more urgent release of Mempool support.

[fbwoolf]

@alter-eggo are you needing more eyes on testing this release or just code review?

[alter-eggo]

@fbwoolf more testing I think

[pete-watters]

If needed to unblock this release today, we can also just add top padding to the receive view in the action popup format?

The lack of consistent top padding there compared to other similar views (e.g. send, swap, buy) is what seems most in need of resolution. We can figure out later if we need to convert the receive view into a page (since it's basically a sheet at the moment, whereas these other views are pages?).

Let's take care to QA PRs on the original PRs themselves going forward. We're unfortunately trying to fix these container related issues now to unblock the more urgent release of Mempool support.

Thanks for your help checking this @markmhendrickson 👍 Also thanks a lot to @alter-eggo for adding so many fixes here, I appreciate it.

In future, if we need to do a release and issues are found like this, we should remove the offending commits from the release branch so it's not blocked rather than having it block more urgent things.

I am checking through the updates made and will add some better tests to catch issues next time.

• this problem with the settings option: release: nostalgic skunk #5726 (comment) was noticed as Settings was previously not available if the user has no wallet. I agree it is a good enhancement to get it working but it was a mistake by me to show the button there
• I will add this enhancement to show the balance in STX - release: nostalgic skunk #5726 (comment)
• you are correct Mark on this discussion around the Back button release: nostalgic skunk #5726 (comment). It's supposed to be a back button in action popup for consistency across flows. I will try and add a test to assert that it shows up on smaller screen sizes