-
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
[HOLD for payment 2025-01-17] [$250] Web - Emoji- Black box for flag emoji are difficult to cancel with backspace key #51336
Comments
Triggered auto assignment to @muttmuure ( |
@muttmuure FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors |
This does not have a BZ assigned. Can you remove and re-add the Bug label? |
Triggered auto assignment to @Christinadobrzyn ( |
@c3024 Done |
ProposalPlease re-state the problem that we are trying to solve in this issue.Flag emojis are not completely deleted with backspace key What is the root cause of that problem?Some flags are composed of multiple unicode points. For example, Wales flag has When we click on backspace key, the last unicode code point is removed which is the Further backspace key presses remove the unicode points one by one. Until the first tag is removed, the sequence stays as an incomplete sequence and it appears that the black flag could be deleted only after pressing backspace several times. What changes do you think we should make in order to solve the problem?Different flags have different number of unicode code points. So, we should use the We will add a new module const triggerHotkeyActions = useCallback(
(event: NativeSyntheticEvent<TextInputKeyPressEventData>) => {
const webEvent = event as unknown as KeyboardEvent;
if (webEvent.key === 'Backspace') {
if (selection.start === 0) {
return;
}
if (selection.start !== selection.end) {
return;
}
const graphemes = splitter.splitGraphemes(lastTextRef.current.substring(0, selection.start));
const lastGrapheme = graphemes[graphemes.length - 1];
if (lastGrapheme.length > 1) {
event.preventDefault();
const newText = lastTextRef.current.slice(0, selection.start - lastGrapheme.length) + lastTextRef.current.slice(selection.start);
setSelection((prevSelection) => ({
start: selection.start - lastGrapheme.length,
end: selection.start - lastGrapheme.length,
positionX: prevSelection.positionX,
positionY: prevSelection.positionY,
}));
updateComment(newText, true);
}
} What alternative solutions did you explore? (Optional) |
gonna investigate this tomorrow when I'm back from ooo |
I don't see this happening on the version v9.0.54-11 (staging) or v9.0.53-1 (prod). I'll see if we can get QA to restest on the latest version. https://expensify.slack.com/archives/C9YU7BX5M/p1730175293093429 |
This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989 |
Still reproducible on latest build v9.0.55-2 bandicam.2024-10-29.14-23-06-918.mp4 |
I'm not sure how to test that version, I don't have that version. I'm asking Tom for some guidance on how to test this |
Asking @izarutskaya for some guidance on how to test this bug in the version they see it in - https://expensify.slack.com/archives/C9YU7BX5M/p1730357190129519?thread_ts=1730175293.093429&cid=C9YU7BX5M |
Job added to Upwork: https://www.upwork.com/jobs/~021852238123831178456 |
Looks like the PR is under review - #52995 |
@eVoloshchak , please check this PR #52995. Thanks! |
monitoring PR #52995 |
monitoring #52995 |
@eVoloshchak , gentle bump for the PR review! |
Dmd @eVoloshchak for a review! |
PR is getting close (in staging)! #52995 |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.82-12 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2025-01-17. 🎊 For reference, here are some details about the assignees on this issue:
|
@eVoloshchak / @c3024 @Christinadobrzyn @eVoloshchak / @c3024 The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button] |
@eVoloshchak or @c3024 can you confirm if we need a regression test for this? TY! |
@eVoloshchak, @Gonals, @Christinadobrzyn, @c3024 Whoops! This issue is 2 days overdue. Let's get this updated quick! |
BugZero Checklist:
Bug classificationSource of bug:
Where bug was reported:
Who reported the bug:
Regression Test Proposal Template
Regression Test ProposalPrecondition:Test:
Do we agree 👍 or 👎 |
Regression test created - https://github.com/Expensify/Expensify/issues/463331 Contributor: @c3024 paid $250 via Upwork (https://www.upwork.com/nx/wm/workroom/39052036/overview) @eVoloshchak please make sure to request payment - closing this out as complete! |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: v9.0.55-2
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): bezijoy@gmail.com
Issue reported by: Applause - Internal Team
Action Performed:
Expected Result:
Any emojis can be erased by backspace key
Actual Result:
Black box flag emojis are difficult to cancel with backspace key (removed from composed box after taping backspace 7 times)
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6643336_1729695740979.bandicam_2024-10-23_17-57-18-262.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @ChristinadobrzynThe text was updated successfully, but these errors were encountered: