Skip to content

Conversation

@cermakjiri
Copy link
Contributor

@cermakjiri cermakjiri commented Nov 25, 2025

Description

refactor: global receive/send fixups

  • remove fiat rates from global receive
  • add search by account index
  • adjust list item spacing
  • update help url in global receive
  • remove hover state of icon in select (network filter) and fix text color based on design

Related Issue

Resolve #22678
Resolve #22677
Resolve #23261

Screenshots:

Snímek obrazovky 2025-11-25 v 14 49 25 Snímek obrazovky 2025-11-25 v 14 34 36 Snímek obrazovky 2025-11-25 v 14 22 42

🔍🖥️ Suite web test results: View in Currents

🔍🖥️ Suite desktop test results: View in Currents

🔍🖥️ Suite native android test results: View in Currents

box-shadow: unset;
width: 100%;
width: calc(100% - ${spacings.xxs * 2}px);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not necessary. Just add the items in a container with padding?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I think the padding of the modal content should match the horizontal padding in the modal header.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It'll be this then:

image

Also, the scroll bar won't placed by the right edge.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I'll try something then:)

Copy link
Contributor

Choose a reason for hiding this comment

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

Here it is: #23388

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, I'll check it out then. Mainly, do not merge it else it will block the feature development. There would be conflicts in my upcoming branches. 🙏

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure there'd be conflicts? My PR is built on top of this branch and handles more or less the same issues, i.e. looks

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you gonna do anything else on top of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I said before: yes and I've already done: #23314.

Btw there's known perf. issue with current implementation of the virtualized list (if there's a lot of items the rendering is super slow) as mentioned here. #23380 (comment). Any improvement would be very welcomed.

@cermakjiri cermakjiri requested review from a team and removed request for a team November 27, 2025 08:55
cursor: pointer;
padding: ${spacingsPx.sm} 0;
padding: ${spacingsPx.xs} 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

padding-block: ${spacingsPx.xs};

Copy link
Contributor

Choose a reason for hiding this comment

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

This is gonna be changed anyway, see #23388

cursor: pointer;
padding: ${spacingsPx.sm} 0;
padding: ${spacingsPx.xs} 0;
margin: ${spacingsPx.xxs} ${spacingsPx.xxs};
Copy link
Contributor

Choose a reason for hiding this comment

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

margin: ${spacingsPx.xxs};

Copy link
Contributor

Choose a reason for hiding this comment

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

The same as above, #23388

@cermakjiri cermakjiri force-pushed the fix/22737-global-receive-send branch from 9f4d270 to 08cff93 Compare November 28, 2025 08:31
@trezor-bot
Copy link
Contributor

trezor-bot bot commented Nov 28, 2025

✅ 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 Nov 28, 2025

✅ 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)

@cermakjiri cermakjiri force-pushed the fix/22737-global-receive-send branch from 08cff93 to 846be8b Compare November 28, 2025 08:54
- remove fiat rates from global receive
- add search by account index
- adjust list item spacing
- update help url in global receive
- remove hover state of icon in select (network filter) and fix text color based on design
@cermakjiri cermakjiri force-pushed the fix/22737-global-receive-send branch from 846be8b to 950adce Compare November 28, 2025 09:12
@cermakjiri
Copy link
Contributor Author

cermakjiri commented Nov 28, 2025

  1. Fix undefined params in back route (i.e. no items were shown due to undefined networkSymbol):
Zaznam.obrazovky.2025-11-28.v.10.14.11.mov
  1. Improve accounts search
Zaznam.obrazovky.2025-11-28.v.10.14.24.mov
  1. Fix resolving coingeckoId in AssetRowToken
  2. Make some perf. improvements of the virtualized list however there's still huge space for bigger refactor
    • I'd suggest get rid of the custom implementation and use library for it (as there are many good of them), e.g. react-window.
    • Or use better browser APIs and re-work the logic to be more efficient. I think the handleScroll could be replaced with more efficient intersection observe API
    • The rest I didn't have time to think through of but I very confident this is not the best way of doing it.
    • The amount of DOM changes is insane. We should only change item visibility and position, not the height. We should cache already rendered items, etc. Anyway, I think easiest would be using already proved solution by some library.

@cermakjiri cermakjiri enabled auto-merge (rebase) November 28, 2025 10:35
@cermakjiri cermakjiri merged commit 3a107c8 into develop Nov 28, 2025
42 of 45 checks passed
@cermakjiri cermakjiri deleted the fix/22737-global-receive-send branch November 28, 2025 10:39
@EuryShadow
Copy link

On smaller screens, the add account button is partially or completely hidden.

Screen.Recording.2025-12-02.at.12.23.46.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Global send & receive updates Global receive Global send

5 participants