Skip to content

[$250] Composer - Composer box is refocused when Copying Link on Web and Desktop #45204

Closed

Description

@lanitochka17

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: 9.0.6-0
Reproducible in staging?: Y
Reproducible in production?: Y
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): applausetester+en@applause.expensifail.com
Issue reported by: Applause - Internal Team

Issue found when executing PR #44399

Action Performed:

  1. Right click on a message (in web or desktop)
  2. Select copy to clipboard

Expected Result:

User doesn't expect the composer to get refocused.

Actual Result:

Composer gets refocussed: Expected chart

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6538235_1720636352340.Composer_is_not_refocused.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01a3374b61f565b9cf
  • Upwork Job ID: 1811154850298352037
  • Last Price Increase: 2024-10-02
Issue OwnerCurrent Issue Owner: @situchan

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Jul 10, 2024
  2. melvin-bot commented on Jul 10, 2024

    @melvin-bot

    Triggered auto assignment to @lschurr (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.

  3. lanitochka17 commented on Jul 10, 2024

    @lanitochka17
    Author

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

  4. lanitochka17 commented on Jul 10, 2024

    @lanitochka17
    Author

    We think that this bug might be related to #vip-vsb

  5. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Jul 10, 2024
  6. changed the title [-]Composer - Composer box is not refocused when Copying Link[/-] [+][$250] Composer - Composer box is not refocused when Copying Link[/+] on Jul 10, 2024
  7. melvin-bot commented on Jul 10, 2024

    @melvin-bot
  8. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Jul 10, 2024
  9. melvin-bot commented on Jul 10, 2024

    @melvin-bot

    Triggered auto assignment to Contributor-plus team member for initial proposal review - @situchan (External)

  10. gijoe0295 commented on Jul 10, 2024

    @gijoe0295
    Contributor

    I'm the author of #44399. That's our bad not to note a precondition for the composer to automatically refocus: it must already be focused before interacting with the context menu. So this is not a valid bug considering the current expectations. We can still change the expectations but should note that this is not a regression. Thanks.

  11. nkdengineer commented on Jul 11, 2024

    @nkdengineer
    Contributor

    Edited by proposal-police: This proposal was edited at 2024-08-05 18:25:46 UTC.

    Proposal

    Please re-state the problem that we are trying to solve in this issue.

    The composer is NOT refocused

    What is the root cause of that problem?

    When a report is focused, we set the composer focus callback with shouldFocusForNonBlurInputOnTapOutside as false by default

    ReportActionComposeFocusManager.onComposerFocus((shouldFocusForNonBlurInputOnTapOutside = false) => {
    if ((!willBlurTextInputOnTapOutside && !shouldFocusForNonBlurInputOnTapOutside) || !isFocused) {
    return;

    When we click on the copy link, we call ReportActionComposeFocusManager.focus function to focus on the composer. The difference here is willBlurTextInputOnTapOutside is false in the native platform and true in other platforms and shouldFocusForNonBlurInputOnTapOutside is false by default so the composer is always re-focused on web/desktop and not re-focused on native

    hideContextMenu(true, ReportActionComposeFocusManager.focus);

    What changes do you think we should make in order to solve the problem?

    We should keep track the composer is focused or not when we open the context menu. Then we pass it as payload data when we open the context menu.

    1. In ReportActionComposeFocusManager, create a new variable to keep track of the time the composer is blurred. And create a function to check this. We have a check time - (composerBluredTime ?? 0) <= 100 because when we open the context menu, the composer is blurred and then the context menu is opened immediately.
    let composerBluredTime: number | null = null;
    function blur() {
        composerBluredTime = new Date().getTime();
    }
    
    function isJustBlured(time: number) {
        return time - (composerBluredTime ?? 0) <= 100;
    }
    
    1. Calling ReportActionComposeFocusManager.blur() here when the composer is blurred
    ReportActionComposeFocusManager.blur()
    

    1. Add an extra param isComposerFocused here
    isComposerFocused = false,
    

    1. In PopoverReportActionContextMenu, get this param here and pass it to BaseReportActionContextMenu

    1. Pass isComposerFocused as a payload data here

    const payload: ContextMenuActionPayload = {

    1. Pass isComposerFocused param as ReportActionComposeFocusManager.isJustBlured(new Date().getTime()) || ReportActionComposeFocusManager.isFocused() when we open the context menu here.
    false,
    ReportActionComposeFocusManager.isJustBlured(new Date().getTime()) || ReportActionComposeFocusManager.isFocused(),
    

    setIsEmojiPickerActive as () => void,

    1. Get isComposerFocused here and check if it's true we will
    hideContextMenu(true, () => {
        if (!isComposerFocused) {
            return;
        }
        ReportActionComposeFocusManager.focus(true);
    });
    

    onPress: (closePopover, {reportAction, reportID}) => {

    The test branch here https://github.com/nkdengineer/App/tree/fix/45204

    What alternative solutions did you explore? (Optional)

    If we want to not focus on the composer after we select the context menu action in mWeb, we can add shouldFocusInputOnScreenFocus check here.

    if ((!willBlurTextInputOnTapOutside && !shouldFocusForNonBlurInputOnTapOutside && !shouldFocusInputOnScreenFocus) || !isFocused) {
        return;
    }
    

    ReportActionComposeFocusManager.onComposerFocus((shouldFocusForNonBlurInputOnTapOutside = false) => {
    if ((!willBlurTextInputOnTapOutside && !shouldFocusForNonBlurInputOnTapOutside) || !isFocused) {
    return;

    shouldFocusInputOnScreenFocus is used to consistent the focus behavior mWeb and native so we can use this

    // We want consistent auto focus behavior on input between native and mWeb so we have some auto focus management code that will
    // prevent auto focus on existing chat for mobile device
    const shouldFocusInputOnScreenFocus = canFocusInputOnScreenFocus();

  12. situchan commented on Jul 12, 2024

    @situchan
    Contributor
    Platform Focused before menu Focused after copying link (before fix) Focused after copying link (after fix)
    Web ✅ ✅
    Web ❌ ✅
    mSafari ✅ ✅
    mSafari ❌ ✅
    mChrome ✅ ✅
    mChrome ❌ ✅
    iOS ✅ ✅
    iOS ❌ ❌ (bug to fix) ✅
    Android ✅ ✅ (but no keyboard popup)
    Android ❌ ❌ (bug to fix) ✅

    So the expected behavior is to refocus always on all platforms regardless of pre-focus status?

  13. 104 remaining items

  14. melvin-bot commented on Sep 18, 2024

    @melvin-bot

    📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

  15. melvin-bot commented on Sep 25, 2024

    @melvin-bot

    📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

  16. melvin-bot commented on Sep 27, 2024

    @melvin-bot

    @lschurr, @srikarparsi, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

  17. lschurr commented on Oct 1, 2024

    @lschurr
    Contributor

    I'm OOO until next week - Did we come to a decision about this one @srikarparsi?

  18. melvin-bot commented on Oct 1, 2024

    @melvin-bot

    @lschurr, @srikarparsi, @situchan 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

  19. srikarparsi commented on Oct 1, 2024

    @srikarparsi
    Contributor

    Just followed up in the thread

  20. melvin-bot commented on Oct 2, 2024

    @melvin-bot

    📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

  21. melvin-bot commented on Oct 3, 2024

    @melvin-bot

    @lschurr, @srikarparsi, @situchan 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

  22. mallenexpensify commented on Oct 3, 2024

    @mallenexpensify
    Contributor

    Closing, as we closed #vib-vsb, this is an edge case bug and not a near-term priority.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

BugSomething is broken. Auto assigns a BugZero manager.DailyKSv2ExternalAdded to denote the issue can be worked on by a contributorHelp WantedApply this label when an issue is open to proposals by contributors

Type

No type

Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions