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

[HOLD for payment 2025-01-17] [$250] Web - Emoji- Black box for flag emoji are difficult to cancel with backspace key #51336

Closed
1 of 8 tasks
lanitochka17 opened this issue Oct 23, 2024 · 52 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@lanitochka17
Copy link

lanitochka17 commented Oct 23, 2024

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:

  1. Navigate to http://www.staging.new.expensify.com/
  2. Navigate to a conversation
  3. Open the emoji picker
  4. Scroll down the list to the bottom( to flag emoji)

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?

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

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
  • Upwork Job URL: https://www.upwork.com/jobs/~021852238123831178456
  • Upwork Job ID: 1852238123831178456
  • Last Price Increase: 2024-11-15
  • Automatic offers:
    • c3024 | Contributor | 104988248
Issue OwnerCurrent Issue Owner: @Christinadobrzyn
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 23, 2024
Copy link

melvin-bot bot commented Oct 23, 2024

Triggered auto assignment to @muttmuure (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@lanitochka17 lanitochka17 removed their assignment Oct 23, 2024
@lanitochka17
Copy link
Author

@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

@c3024
Copy link
Contributor

c3024 commented Oct 25, 2024

@lanitochka17

This does not have a BZ assigned. Can you remove and re-add the Bug label?

@lanitochka17 lanitochka17 added Bug Something is broken. Auto assigns a BugZero manager. and removed Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 25, 2024
Copy link

melvin-bot bot commented Oct 25, 2024

Triggered auto assignment to @Christinadobrzyn (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@melvin-bot melvin-bot bot added the Daily KSv2 label Oct 25, 2024
@lanitochka17
Copy link
Author

@c3024 Done

@c3024
Copy link
Contributor

c3024 commented Oct 25, 2024

Proposal

Please 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
• U+E0067 (TAG LATIN SMALL LETTER G)
• U+E0062 (TAG LATIN SMALL LETTER B)
• U+E0077 (TAG LATIN SMALL LETTER W)
• U+E006C (TAG LATIN SMALL LETTER L)
• U+E0073 (TAG LATIN SMALL LETTER S)
• U+E007F (CANCEL TAG)

When we click on backspace key, the last unicode code point is removed which is the CANCEL TAG here. Once this last code point is removed, the sequence becomes an incomplete sequence and the fallback black flag appears.

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 grapheme-spiltter library to identify the graphemes and remove a grapheme on a backspace press.

We will add a new module grapheme-splitter in package.json and import GraphemeSplitter in ComposerWithSuggestions and update the triggerHotkeyActions (name might need to be changed to accurately reflect the new behaviour) to something like this:

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)

@Christinadobrzyn
Copy link
Contributor

gonna investigate this tomorrow when I'm back from ooo

@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented Oct 29, 2024

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

@Christinadobrzyn Christinadobrzyn added the Needs Reproduction Reproducible steps needed label Oct 29, 2024
@MelvinBot
Copy link

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

@izarutskaya
Copy link

Still reproducible on latest build v9.0.55-2

bandicam.2024-10-29.14-23-06-918.mp4

@Christinadobrzyn
Copy link
Contributor

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

@Christinadobrzyn
Copy link
Contributor

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

@Christinadobrzyn Christinadobrzyn added External Added to denote the issue can be worked on by a contributor and removed Needs Reproduction Reproducible steps needed labels Nov 1, 2024
@melvin-bot melvin-bot bot changed the title Web - Emoji- Black box for flag emoji are difficult to cancel with backspace key [$250] Web - Emoji- Black box for flag emoji are difficult to cancel with backspace key Nov 1, 2024
Copy link

melvin-bot bot commented Nov 1, 2024

Job added to Upwork: https://www.upwork.com/jobs/~021852238123831178456

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 1, 2024
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Nov 23, 2024
@Christinadobrzyn
Copy link
Contributor

Looks like the PR is under review - #52995

@c3024
Copy link
Contributor

c3024 commented Dec 5, 2024

@eVoloshchak , please check this PR #52995. Thanks!

@Christinadobrzyn
Copy link
Contributor

monitoring PR #52995

@Christinadobrzyn
Copy link
Contributor

monitoring #52995

@c3024
Copy link
Contributor

c3024 commented Dec 18, 2024

@eVoloshchak , gentle bump for the PR review!

@c3024
Copy link
Contributor

c3024 commented Dec 23, 2024

@eVoloshchak 👀

@Christinadobrzyn
Copy link
Contributor

Dmd @eVoloshchak for a review!

@Christinadobrzyn
Copy link
Contributor

PR is getting close (in staging)! #52995

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jan 10, 2025
@melvin-bot melvin-bot bot changed the title [$250] Web - Emoji- Black box for flag emoji are difficult to cancel with backspace key [HOLD for payment 2025-01-17] [$250] Web - Emoji- Black box for flag emoji are difficult to cancel with backspace key Jan 10, 2025
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jan 10, 2025
Copy link

melvin-bot bot commented Jan 10, 2025

Reviewing label has been removed, please complete the "BugZero Checklist".

Copy link

melvin-bot bot commented Jan 10, 2025

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:

Copy link

melvin-bot bot commented Jan 10, 2025

@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]

@garrettmknight garrettmknight moved this from Bugs and Follow Up Issues to Hold for Payment in [#whatsnext] #expense Jan 14, 2025
@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jan 16, 2025
@Christinadobrzyn
Copy link
Contributor

@eVoloshchak or @c3024 can you confirm if we need a regression test for this? TY!

@melvin-bot melvin-bot bot added the Overdue label Jan 20, 2025
Copy link

melvin-bot bot commented Jan 20, 2025

@eVoloshchak, @Gonals, @Christinadobrzyn, @c3024 Whoops! This issue is 2 days overdue. Let's get this updated quick!

@c3024
Copy link
Contributor

c3024 commented Jan 20, 2025

BugZero Checklist:

  • [Contributor] Classify the bug:
Bug classification

Source of bug:

  • 1a. Result of the original design (eg. a case wasn't considered)
  • 1b. Mistake during implementation
  • 1c. Backend bug
  • 1z. Other:

Where bug was reported:

  • 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
  • 2b. Reported on staging (eg. found during regression or PR testing)
  • 2d. Reported on a PR
  • 2z. Other:

Who reported the bug:

  • 3a. Expensify user
  • 3b. Expensify employee
  • 3c. Contributor
  • 3d. QA
  • 3z. Other:
  • [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

    Link to comment: This is an edge case and no PR can be held responsible.

  • [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

    Link to discussion: No discussion was started because this does not interrupt a core flow.

  • [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

Regression Test Proposal Template

Regression Test Proposal

Precondition:

Test:

  1. [Desktop Chrome] Input Wales flag emoji ( 🏴󠁧󠁢󠁷󠁬󠁳󠁿 )in composer.
  2. Press backspace.
  3. Verify that the flag emoji is completely removed from the composer.

Do we agree 👍 or 👎

@Christinadobrzyn
Copy link
Contributor

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)
Contributor+: @eVoloshchak owed $250 via NewDot

@eVoloshchak please make sure to request payment - closing this out as complete!

@github-project-automation github-project-automation bot moved this from Hold for Payment to Done in [#whatsnext] #expense Jan 21, 2025
@melvin-bot melvin-bot bot removed the Overdue label Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
Status: Done
Development

No branches or pull requests

9 participants