Skip to content
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.

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

feat: Simple send modal recipient view #17096

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Cuteivist
Copy link
Contributor

@Cuteivist Cuteivist commented Jan 21, 2025

Closes #16916

User stories (during the update): https://www.notion.so/Recipient-selection-12e8f96fb65c80cebb0deb75533fa255

What does the PR do

  • Added RecipientViewAdaptor to transform 3 models into 2 models used in recipient view
  • Simplified tab delegates and input to use the same component
  • Added new filtering logic
  • Updated input verification rules

Affected areas

  • Send modal

Architecture compliance

Screenshot of functionality (including design for comparison)

recipient_view.mov

@Cuteivist Cuteivist requested a review from a team as a code owner January 21, 2025 07:03
@Cuteivist Cuteivist force-pushed the feat/recipient-selector-19916 branch from ec9deff to 27dcfd2 Compare January 21, 2025 07:06
@status-im-auto
Copy link
Member

status-im-auto commented Jan 21, 2025

Jenkins Builds

Click to see older builds (7)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 27dcfd2 #2 2025-01-21 07:13:42 ~7 min macos/aarch64 🍎dmg
✔️ 27dcfd2 #2 2025-01-21 07:15:30 ~8 min tests/nim 📄log
27dcfd2 #2 2025-01-21 07:18:48 ~12 min tests/ui 📄log
✔️ 27dcfd2 #2 2025-01-21 07:21:59 ~15 min macos/x86_64 🍎dmg
✔️ 27dcfd2 #2 2025-01-21 07:23:49 ~17 min linux-nix/x86_64 📦tgz
✔️ 27dcfd2 #2 2025-01-21 07:28:53 ~22 min linux/x86_64 📦tgz
✔️ 27dcfd2 #2 2025-01-21 07:35:32 ~28 min windows/x86_64 💿exe
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 8dd3ea0 #4 2025-01-22 06:31:33 ~5 min macos/aarch64 🍎dmg
✔️ 8dd3ea0 #4 2025-01-22 06:33:52 ~8 min tests/nim 📄log
8dd3ea0 #4 2025-01-22 06:38:27 ~12 min tests/ui 📄log
✔️ 8dd3ea0 #4 2025-01-22 06:41:56 ~16 min linux-nix/x86_64 📦tgz
✔️ 8dd3ea0 #4 2025-01-22 06:42:06 ~16 min linux/x86_64 📦tgz
✔️ 8dd3ea0 #4 2025-01-22 06:42:18 ~16 min macos/x86_64 🍎dmg
✔️ 6158b50 #5 2025-01-22 06:53:34 ~4 min macos/aarch64 🍎dmg
✔️ 6158b50 #5 2025-01-22 06:56:54 ~7 min tests/nim 📄log
✔️ 6158b50 #5 2025-01-22 07:00:22 ~11 min macos/x86_64 🍎dmg
✔️ 6158b50 #5 2025-01-22 07:01:14 ~12 min tests/ui 📄log
✔️ 6158b50 #5 2025-01-22 07:04:04 ~15 min linux-nix/x86_64 📦tgz
✔️ 6158b50 #5 2025-01-22 07:06:01 ~17 min linux/x86_64 📦tgz
✔️ 6158b50 #5 2025-01-22 07:15:50 ~26 min windows/x86_64 💿exe

Copy link
Member

@caybro caybro left a comment

Choose a reason for hiding this comment

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

LGTM in general, massive work!

Just some hints to get rid of the slow SFPM stuff

ui/app/AppLayouts/Wallet/adaptors/RecipientViewAdaptor.qml Outdated Show resolved Hide resolved
ui/app/AppLayouts/Wallet/adaptors/RecipientViewAdaptor.qml Outdated Show resolved Hide resolved
if (sensor.containsMouse) {
return root.address
}
return elideAddress ? StatusQUtils.Utils.elideText(root.address, 6, 4) : root.address
Copy link
Member

Choose a reason for hiding this comment

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

We have some dedicated "elide wallet address" function imo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used this one, because the dedicated one uses specific character to display "x", which doesn't display correctly on indenticon.

Added comment here

@Cuteivist Cuteivist force-pushed the feat/recipient-selector-19916 branch 2 times, most recently from 13a260f to 8dd3ea0 Compare January 22, 2025 06:25
@Cuteivist Cuteivist force-pushed the feat/recipient-selector-19916 branch from 8dd3ea0 to 6158b50 Compare January 22, 2025 06:48
@Cuteivist
Copy link
Contributor Author

@caybro updated all your remarks. Thanks for improvement ideas! Please have a look again.

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.

Recipient selection
3 participants