Skip to content

chore(suite): show all accounts in global send modal #23586

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
tomasklim merged 1 commit into from
Dec 8, 2025
Merged

chore(suite): show all accounts in global send modal #23586

tomasklim merged 1 commit into from
Dec 8, 2025
+48 −55

Conversation

@matusbalascak
Copy link
Contributor

@matusbalascak matusbalascak commented Dec 3, 2025 •
edited by tomasklim
Loading

Description

  • Global send modal "Activate {network}" button now has a loading state while the discovery is running
  • All accounts are now included in global send modal, not just accounts with balance
  • Changed button priority to secondary for ConditionalActionRenderer

Notes for QA

  • All accounts are visible in the global send modal
  • "Activate {network}" button in global send modal now has a loading state while there is a running discovery
  • validate all types of btc account types because some types are case sensitive

Related Issue

Resolve

Screenshots:

Notification - changed button priority to secondary

image

Global send modal - change "Activate {network}" button priority to secondary

image

Flow

fdsafdsafdsa.mov

🔍🖥️ Suite web test results: View in Currents

🔍🖥️ Suite desktop test results: View in Currents

@matusbalascak matusbalascak requested review from a team and tomasklim December 3, 2025 09:17
@matusbalascak matusbalascak moved this to 🔎 Needs review in Suite Dec 3, 2025
izmy
izmy approved these changes Dec 3, 2025
View reviewed changes
packages/suite/src/components/suite/asset-picker/components/AssetsListEmpty/AssetsListEmpty.tsx
const dispatch = useDispatch();
const protocolScheme = useSelector(state => state.protocol.sendForm.scheme);
const device = useSelector(selectSelectedDevice);
const isDiscoveryRunning = useSelector(selectHasRunningDiscovery);
Copy link
Contributor

@izmy izmy Dec 3, 2025

Choose a reason for hiding this comment

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

I just noticed the weird naming isDiscoveryRunning x selectHasRunningDiscovery, but it's used throughout the codebase, so keep it consistent 😄

@matusbalascak matusbalascak force-pushed the chore/global-send-nits branch from 3bae0e6 to ef8b60b Compare December 4, 2025 14:26
@trezor-bot
Copy link
Contributor

trezor-bot bot commented Dec 4, 2025 •
edited
Loading

✅ Previously successful run of [Test] PR Suite Desktop e2e tests workflow has been found.
⏭️ Skipping tests for this run.
💡 If you are unsure about your latest changes, please rerun the workflow manually. (Use the Re-run all jobs option)

@trezor-bot
Copy link
Contributor

trezor-bot bot commented Dec 4, 2025 •
edited
Loading

✅ Previously successful run of [Test] PR Suite Web e2e tests workflow has been found.
⏭️ Skipping tests for this run.
💡 If you are unsure about your latest changes, please rerun the workflow manually. (Use the Re-run all jobs option)

tomasklim
tomasklim requested changes Dec 4, 2025
View reviewed changes
packages/suite/src/hooks/wallet/useSendForm.ts Outdated
if (shouldFillFromProtocol) {
const values = getLoadedValues();
values.outputs[0].address = protocol.sendForm.address!;
values.outputs[0].address = protocol.sendForm.address?.toLowerCase() ?? '';
Copy link
Member

@tomasklim tomasklim Dec 4, 2025

Choose a reason for hiding this comment

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

legacy bitcoin addresses are case sensitive, so you cannot do that

For BTC addresses are case insensitive for P2TR and P2WPKH while P2SH and P2PKH addresses ARE case sensitive.

tomasklim
tomasklim reviewed Dec 4, 2025
View reviewed changes
...uiteLayout/PageHeader/GlobalSendReceive/GlobalSendModal/hooks/useAccountWithTokensOptions.ts
return networkSymbol ? findAccountsByNetwork(networkSymbol, accounts) : accounts;
}

function getAccountsWithPositiveBalanceOrVisibleTokens(
Copy link
Member

@tomasklim tomasklim Dec 4, 2025

Choose a reason for hiding this comment

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

why?

Copy link
Contributor Author

@matusbalascak matusbalascak Dec 5, 2025

Choose a reason for hiding this comment

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

There was a case with #14700 where if a user was asked to activate the network in a global send modal and after doing it he didn't have any account with balance, he would just see the empty list. I spoke to Jemel and we agreed that it would be better to show also 0 balance accounts there to prevent this case.

@matusbalascak matusbalascak force-pushed the chore/global-send-nits branch from ef8b60b to d525a61 Compare December 6, 2025 11:39
@tomasklim tomasklim merged commit 5382897 into develop Dec 8, 2025
50 checks passed
@tomasklim tomasklim deleted the chore/global-send-nits branch December 8, 2025 12:49
@github-project-automation github-project-automation bot moved this from 🔎 Needs review to 🤝 Needs QA in Suite Dec 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@izmy izmy izmy approved these changes

@tomasklim tomasklim tomasklim approved these changes

Assignees

No one assigned

Labels

cherry-pick🍒

Projects

Suite
Status: 🤝 Needs QA

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@matusbalascak @izmy @tomasklim