-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fix numpad longpress back key #40288
Fix numpad longpress back key #40288
Conversation
@jayeshmangwani Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
@FitseTLT Upload video for missing platforms, Desktop, web, mWeb safari, and mWeb chrome |
Long-pressing in mWeb Safari and mWeb Chrome works differently than native long press (long press in mWeb does nothing). Can we solve that issue here? |
There is no big numpad for desktop and web
The problem is that longpress doesn't work at all on mWeb when you press and hold it will release it automatically so if it is to be fixed not at all in the scope of this issue. |
Still, we should record and upload videos of desktop and web using the onscreen keyboard
It's happening on staging, too, so let's confirm with the internal engineer. @chiragsalian In mobile web, the long press is not working for the number pad(it deletes one character only), so should we open another issue to handle that? |
Yeah that sounds like a different bug to me so let's open another issue for it. This issue is mainly focused on mobile and long press functionality is working different than expected. On mobile web it sounds like right now its not possible to long press so i think we should open up another issue to discuss its important and then get its behavior consistent with mobile. |
Added Snapshots |
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid.movAndroid: mWeb ChromemWeb-chrome.moviOS: NativeiOS.moviOS: mWeb SafarimWeb-safari.movMacOS: Chrome / Safariweb.movMacOS: Desktopdesktop.mov |
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.
Code changes LGTM 🚀
NAB: @FitseTLT Please update the Tests and include that the long press will work for native devices, and for mobile web, the last digit only will Be deleted when a long press
🚀 Deployed to staging by https://github.com/chiragsalian in version: 1.4.65-0 🚀
|
🚀 Deployed to production by https://github.com/mountiny in version: 1.4.65-5 🚀
|
Details
Fixed Issues
$ #39079
PROPOSAL: #39079 (comment)
Tests
Precondition:
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
and.mp4
Android: mWeb Chrome
and.web.mp4
iOS: Native
ios.mp4
iOS: mWeb Safari
2024-04-20.16-50-07.mp4
MacOS: Chrome / Safari
eb.mp4
MacOS: Desktop
d.mp4