-
Notifications
You must be signed in to change notification settings - Fork 985
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
[#19265] fix: cursor overlapping placeholder in android #19526
Conversation
Jenkins BuildsClick to see older builds (16)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @mohsen-ghafouri thanks for fixing this.
@status-im/mobile-qa i don't know if design feedback's issues require QA or we can directly move to design review step. please advice? |
0% of end-end tests have passed
Failed tests (47)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePRTwo:
Class TestDeepLinksOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
|
@mohsen-ghafouri thanks for the PR. Design feedback will be enough for this PR. Just ping us to review e2e when results are ready. |
@mohsen-ghafouri BTW, I see there is a conflict in src/quo/components/inputs/input/view.cljs Could you please resolve? Thank you! |
wow, I see all e2e are failed, so it definitely needs review. @yevh-berdnyk could you please take a look when you have a chance? Thank you! |
a1df6cb
to
1298def
Compare
Oh i see, @yevh-berdnyk if some testes are depend on input placeholder, in android i add half empty space before placeholder text to resolve cursor overlap |
Ok, I'll update the tests and rerun them |
90% of end-end tests have passed
Failed tests (4)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestWalletMultipleDevice:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (47)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestDeepLinksOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestWalletOneDevice:
|
@yevh-berdnyk thank you for tests fixes. @mohsen-ghafouri please include Yeveniia's commit during merge. PR is ready for design review cc @Francesca-G |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fixed 👍
652d60a
to
1cab726
Compare
@Francesca-G can i move it to Merge step? |
@mohsen-ghafouri I think we can move to merge if design approves. Done. |
1cab726
to
09ac8b1
Compare
09ac8b1
to
3e08a74
Compare
was not included 😥 |
@yevh-berdnyk could you please push your fix into develop, as currently e2e in all rebased builds are failing. |
Oh sorry my bad @pavloburykh, missed in the last rebase 🥲 |
no worries, Yevheniia will push a fix soon. |
fixes #19265
Summary
Placeholder [comment] in the input field overlaps with the typing cursor. This happened in the Add a contact screen
Technical note
as i tested the issue happens when we set
placeholder-text-color
in android. workaround for this i have to add a tiny empty space before placeholder to eliminate this overlapfacebook/react-native#27687
Platforms
Areas that maybe impacted
Steps to test
Result
status: ready