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

Quo - truncation for component wallet/account card fix truncation #18256

Closed
wants to merge 10 commits into from

Conversation

Pau1fitz
Copy link
Collaborator

@Pau1fitz Pau1fitz commented Dec 20, 2023

Quo - If the name is too long on a wallet account text should be truncated using an ellipsis.

Fixes #18044

Figma example

Before:
Screenshot 2023-12-21 at 19 09 56

After:
Screenshot 2023-12-20 at 14 10 16

Testing notes:
Create a new wallet account with a really long name
Go to wallet home page
Observe wallet card uses ellipsis as in the designs.

Can also be tested in the Quo preview screen for wallet -> account card component

Can be tested on release builds here:
#18256 (comment)

@ghost
Copy link

ghost commented Dec 20, 2023

Hey @Pau1fitz, and thank you so much for making your first pull request in status-mobile! ❤️ Please help us make your experience better by filling out this brief questionnaire https://goo.gl/forms/uWqNcVpVz7OIopXg2

Copy link
Contributor

@siddarthkay siddarthkay left a comment

Choose a reason for hiding this comment

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

looks good

Copy link
Member

@smohamedjavid smohamedjavid left a comment

Choose a reason for hiding this comment

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

Thank you for the fix! @Pau1fitz 🚀

@J-Son89
Copy link
Contributor

J-Son89 commented Dec 20, 2023

@status-im/mobile-qa - this can be tested here: #18259

@Pau1fitz
Copy link
Collaborator Author

@smohamedjavid @OmarBasem @siddarthkay @J-Son89

is the below a known issue? When you type the keyboard doesn't expand.

Screen.Recording.2023-12-20.at.17.04.55.mov

@smohamedjavid
Copy link
Member

is the below a known issue? When you type the keyboard doesn't expand.

It's the iOS simulator's behaviour when you type using your system (hard) keyboard. The device's (soft) keyboard can be toggled by using cmd (⌘) + K.

@pavloburykh pavloburykh self-assigned this Dec 21, 2023
@pavloburykh
Copy link
Contributor

@Pau1fitz thanx for the PR.

In case of watch only account we need to take into consideration the size of the "eye icon". Otherwise in case of long name it is displaced too close to the wallet card border (see screenshot below). @J-Son89 WDYT? I have not found an example of long name watch only account in Figma.

Below screenshot is taken on iPhone X, IOS 16.4

photo_2023-12-21 18 54 17

@J-Son89
Copy link
Contributor

J-Son89 commented Dec 21, 2023

@Pau1fitz thanx for the PR.

In case of watch only account we need to take into consideration the size of the "eye icon". Otherwise in case of long name it is displaced too close to the wallet card border (see screenshot below). @J-Son89 WDYT? I have not found an example of long name watch only account in Figma.

Below screenshot is taken on iPhone X, IOS 16.4

photo_2023-12-21 18 54 17

good spot @Pau1fitz, yeah should be easy enough to add this. Seems just like some padding or margin should sort it 👍

@Pau1fitz
Copy link
Collaborator Author

after pulling the latest code and running make run-metro I am seeing the following error. Has anyone seen this before?

 ERROR  TypeError: native_module.core.status.call(null).fleets is not a function. (In 'native_module.core.status.call(null).fleets()', 'native_module.core.status.call(null).fleets' is undefined)

@J-Son89
Copy link
Contributor

J-Son89 commented Dec 21, 2023

ah yes, there was a recent breaking change made to Status-Go where fleets changed. You will have to re run make run-ios to rebuild the app

@Pau1fitz
Copy link
Collaborator Author

@pavloburykh ready for re-testing. See screenshots below

Screenshot 2023-12-21 at 19 04 02 Screenshot 2023-12-21 at 19 05 41

@pavloburykh
Copy link
Contributor

@J-Son89 could you please update correspondent #18259 so I can test changes? Thank you.

@pavloburykh
Copy link
Contributor

@J-Son89 nevermind. Sorry, I have forgotten that you are off.

@J-Son89
Copy link
Contributor

J-Son89 commented Dec 22, 2023

No problem @pavloburykh, I had one or two questions about the update I sent to @Pau1fitz. Once I get an answer I'll update the other pr with changes. We can wait with testing until then. Thanks!

@pavloburykh
Copy link
Contributor

@Pau1fitz thank you for the fix. Tested in #18256. Ready for merge.

@pavloburykh
Copy link
Contributor

pavloburykh commented Dec 22, 2023

No problem @pavloburykh, I had one or two questions about the update I sent to @Pau1fitz. Once I get an answer I'll update the other pr with changes. We can wait with testing until then. Thanks!

Oh, okay. Missed this comment @J-Son89 and moved PR to merged already. Ping me up if it will require re-test.

@J-Son89
Copy link
Contributor

J-Son89 commented Dec 22, 2023

Ah no problem, good from my side now too so I'll merge it 👌

@J-Son89 J-Son89 closed this Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Archived in project
Development

Successfully merging this pull request may close these issues.

Quo - fix longname for component wallet/account card
6 participants