[HOLD for payment 2024-03-13] [$500] Workspace – Unable to save edited WS name using Enter key in newly created WS. #36923

lanitochka17 · 2024-02-20T16:49:39Z

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: 1.4.43-3
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4327666
Email or phone of affected tester (no customers): applausetester+jp_e_category@applause.expensifail.com
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

Go to https://staging.new.expensify.com/
Login with account that has at least 1 WS already created
Click on "choose Workspace" icon in top left
Create a new WS by hitting "+"
Edit WS name
Press Enter key to save the edited WS name

Expected Result:

Edited WS name is saved

Actual Result:

Nothing happens

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6385818_1708442372353.Unable_to_save_edited_WS_name_using_Enter_key.1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0109e93e51a9b4c9ed
Upwork Job ID: 1760135649726885888
Last Price Increase: 2024-02-21
Automatic offers:

getusha | Reviewer | 0
bernhardoj | Contributor | 0

melvin-bot · 2024-02-20T16:49:51Z

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

Krishna2323 · 2024-02-20T16:59:23Z

Proposal

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

Workspace – Unable to save edited WS name using Enter key in newly created WS.

What is the root cause of that problem?

We don't pass disablePressOnEnter={false} to FormProvider in WorkspaceNamePage

App/src/pages/workspace/WorkspaceNamePage.tsx

Line 66 in 669ac33

<FormProvider

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

Pass disablePressOnEnter={false} to FormProvider in WorkspaceNamePage

Result

kadiealexander · 2024-02-21T00:55:42Z

Can't reproduce on v1.4.43-5

2024-02-21_13-54-48.mp4

mvtglobally · 2024-02-21T01:47:45Z

Able to reproduce.
Issue is repro ONLY if you already have a WS created. If you are creating 1st one its not repro

Screen.Recording.2024-02-20.at.8.53.51.PM.mov

kadiealexander · 2024-02-21T02:45:36Z

Still unable to repro:

2024-02-21_15-39-18.mp4

kadiealexander · 2024-02-21T02:50:33Z

Ah wait, I clicked the wrong "create a workspace" button. Was able to repro this.

melvin-bot · 2024-02-21T02:53:37Z

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

melvin-bot · 2024-02-21T02:53:42Z

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

bernhardoj · 2024-02-21T03:28:56Z

Proposal

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

The enter key to save the workspace name doesn't work after creating a new workspace from the switcher page.

What is the root cause of that problem?

Summary: The ENTER key is captured by another page.

When we press the new workspace from the switcher page, we will be navigated to the newly created workspace page (profile).

App/src/pages/WorkspaceSwitcherPage.tsx

Lines 220 to 222 in 6989089

    
           onPress={() => { 
        
               App.createWorkspaceWithPolicyDraftAndNavigateToIt(); 
        
           }}

We never pop the switcher page out from the nav stack, so it still exists on the stack. The workspace switcher page uses OptionsSelector to show the list which has its own ENTER key shortcut. We already disable the shortcut if the screen goes out of focus.

App/src/components/OptionsSelector/BaseOptionsSelector.js

Lines 124 to 131 in 6989089

    
           if (prevProps.isFocused !== this.props.isFocused) { 
        
               // Unregister the shortcut before registering a new one to avoid lingering shortcut listener 
        
               this.unSubscribeFromKeyboardShortcut(); 
        
               if (this.props.isFocused) { 
        
                   this.subscribeToEnterShortcut(); 
        
                   this.subscribeToCtrlEnterShortcut(); 
        
               } 
        
           }

But we have another condition to subscribe it back.

App/src/components/OptionsSelector/BaseOptionsSelector.js

Lines 116 to 122 in 6989089

    
           if (prevState.disableEnterShortCut !== this.state.disableEnterShortCut) { 
        
               // Unregister the shortcut before registering a new one to avoid lingering shortcut listener 
        
               this.unsubscribeEnter(); 
        
               if (!this.state.disableEnterShortCut) { 
        
                   this.subscribeToEnterShortcut(); 
        
               } 
        
           }

disableEnterShortcut is set based on focusin and focusout listener.

App/src/components/OptionsSelector/BaseOptionsSelector.js

Lines 269 to 280 in 6989089

    
           handleFocusIn() { 
        
               const activeElement = document.activeElement; 
        
               this.setState({ 
        
                   disableEnterShortCut: activeElement && this.accessibilityRoles.includes(activeElement.role) && activeElement.role !== CONST.ROLE.PRESENTATION, 
        
               }); 
        
           } 
        
           handleFocusOut() { 
        
               this.setState({ 
        
                   disableEnterShortCut: false, 
        
               }); 
        
           }

When we press the new workspace button, focusin is triggered with the new workspace button as the active element. At this time, disableEnterShortcut is true. When the newly created workspace profile page opens, focusout is called, making disableEnterShortcut becomes false. Because disableEnterShortcut is false, the ENTER shortcut key is re-registered even though the screen is not focused anymore.

This only happens when you have at least 1 workspace because if you have none, the options selector won't be rendered.

App/src/pages/WorkspaceSwitcherPage.tsx

Lines 237 to 238 in 6989089

    
           {usersWorkspaces.length > 0 ? ( 
        
               <OptionsSelector

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

We have an unsubs code for the focusin and focusout, but we never use it.

App/src/components/OptionsSelector/BaseOptionsSelector.js

Lines 290 to 296 in 6989089

    
           unSubscribeActiveElement() { 
        
               if (!this.isWebOrDesktop) { 
        
                   return; 
        
               } 
        
               document.removeEventListener('focusin', this.handleFocusIn); 
        
               document.removeEventListener('focusout', this.handleFocusOut); 
        
           }

Unsubs them when the screen is out of focus.

App/src/components/OptionsSelector/BaseOptionsSelector.js

Lines 124 to 131 in 6989089

    
           if (prevProps.isFocused !== this.props.isFocused) { 
        
               // Unregister the shortcut before registering a new one to avoid lingering shortcut listener 
        
               this.unSubscribeFromKeyboardShortcut(); 
        
               if (this.props.isFocused) { 
        
                   this.subscribeToEnterShortcut(); 
        
                   this.subscribeToCtrlEnterShortcut(); 
        
               } 
        
           }

if (this.props.isFocused) {
} else {
    this.unSubscribeActiveElement()
}

What alternative solutions did you explore? (Optional)

Only do the subscription if isFocused is true.

App/src/components/OptionsSelector/BaseOptionsSelector.js

Lines 116 to 122 in 6989089

    
           if (prevState.disableEnterShortCut !== this.state.disableEnterShortCut) { 
        
               // Unregister the shortcut before registering a new one to avoid lingering shortcut listener 
        
               this.unsubscribeEnter(); 
        
               if (!this.state.disableEnterShortCut) { 
        
                   this.subscribeToEnterShortcut(); 
        
               } 
        
           }

OR

Pop the switcher page when creating a new workspace.

App/src/pages/WorkspaceSwitcherPage.tsx

Lines 220 to 222 in 6989089

    
           onPress={() => { 
        
               App.createWorkspaceWithPolicyDraftAndNavigateToIt(); 
        
           }}

I think we should do this regardless of the solution selected.

getusha · 2024-02-21T18:05:38Z

@bernhardoj's proposal (Main solution) looks good to me. works well.
🎀 👀 🎀 C+ Reviewed!

melvin-bot · 2024-02-21T18:05:52Z

Triggered auto assignment to @francoisl, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

melvin-bot · 2024-02-21T18:26:37Z

📣 @getusha 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

melvin-bot · 2024-02-21T18:26:46Z

📣 @bernhardoj 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

bernhardoj · 2024-02-22T05:46:09Z

PR is ready

cc: @getusha

melvin-bot · 2024-03-06T21:45:42Z

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

melvin-bot · 2024-03-06T21:45:44Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.47-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

Fix can't save workspace name using ENTER key after creating a new workspace from workspace switcher page #37069

If no regressions arise, payment will be issued on 2024-03-13. 🎊

For reference, here are some details about the assignees on this issue:

@bernhardoj requires payment automatic offer (Contributor)
@getusha requires payment automatic offer (Reviewer)

melvin-bot · 2024-03-06T21:45:45Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@getusha] The PR that introduced the bug has been identified. Link to the PR: Prevent enter key event if the focused element is a clickable element #32464
[@getusha] 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: https://github.com/Expensify/App/pull/32464/files#r1521968148
[@getusha] A discussion in #expensify-bugs 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: N/a
[@getusha] Determine if we should create a regression test for this bug. Yes
[@getusha] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/378645

kadiealexander · 2024-03-12T03:32:37Z

@getusha don't forget the checklist!

getusha · 2024-03-12T18:53:42Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@getusha] The PR that introduced the bug has been identified. Link to the PR: Prevent enter key event if the focused element is a clickable element #32464
[@getusha] 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: https://github.com/Expensify/App/pull/32464/files#r1521968148
[@getusha] A discussion in #expensify-bugs 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: N/a
[@getusha] Determine if we should create a regression test for this bug. Yes
[@getusha] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

Open workspace switcher page
Create new workspace (+ icon)
Open workspace profile page
Press on the workspace name
Edit the name
Press enter/return
Verify the name is saved and the page is closed

Do we agree 👍 or 👎

kadiealexander · 2024-03-13T22:05:26Z

Payouts due:

Contributor: $500 @bernhardoj
Contributor+: $500 @getusha

Upwork job is here.

lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 20, 2024

melvin-bot bot assigned kadiealexander Feb 20, 2024

kadiealexander added the External Added to denote the issue can be worked on by a contributor label Feb 21, 2024

melvin-bot bot changed the title ~~Workspace – Unable to save edited WS name using Enter key in newly created WS.~~ [$500] Workspace – Unable to save edited WS name using Enter key in newly created WS. Feb 21, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 21, 2024

melvin-bot bot assigned getusha Feb 21, 2024

melvin-bot bot assigned francoisl Feb 21, 2024

francoisl assigned bernhardoj Feb 21, 2024

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 21, 2024

bernhardoj mentioned this issue Feb 22, 2024

Fix can't save workspace name using ENTER key after creating a new workspace from workspace switcher page #37069

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Feb 22, 2024

melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Mar 6, 2024

melvin-bot bot added the Awaiting Payment Auto-added when associated PR is deployed to production label Mar 6, 2024

melvin-bot bot changed the title ~~[$500] Workspace – Unable to save edited WS name using Enter key in newly created WS.~~ [HOLD for payment 2024-03-13] [$500] Workspace – Unable to save edited WS name using Enter key in newly created WS. Mar 6, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Mar 6, 2024

kadiealexander added Daily KSv2 and removed Weekly KSv2 labels Mar 12, 2024

getusha mentioned this issue Mar 12, 2024

Prevent enter key event if the focused element is a clickable element #32464

Merged

50 tasks

melvin-bot bot added Daily KSv2 and removed Daily KSv2 labels Mar 13, 2024

kadiealexander closed this as completed Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-03-13] [$500] Workspace – Unable to save edited WS name using Enter key in newly created WS. #36923

[HOLD for payment 2024-03-13] [$500] Workspace – Unable to save edited WS name using Enter key in newly created WS. #36923

lanitochka17 commented Feb 20, 2024 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Feb 20, 2024

Krishna2323 commented Feb 20, 2024 •

edited

Loading

kadiealexander commented Feb 21, 2024

mvtglobally commented Feb 21, 2024 •

edited

Loading

kadiealexander commented Feb 21, 2024

kadiealexander commented Feb 21, 2024

melvin-bot bot commented Feb 21, 2024

melvin-bot bot commented Feb 21, 2024

bernhardoj commented Feb 21, 2024

getusha commented Feb 21, 2024

melvin-bot bot commented Feb 21, 2024

melvin-bot bot commented Feb 21, 2024

melvin-bot bot commented Feb 21, 2024

bernhardoj commented Feb 22, 2024

melvin-bot bot commented Mar 6, 2024

melvin-bot bot commented Mar 6, 2024

melvin-bot bot commented Mar 6, 2024 •

edited by kadiealexander

Loading

kadiealexander commented Mar 12, 2024

getusha commented Mar 12, 2024

kadiealexander commented Mar 13, 2024 •

edited

Loading

[HOLD for payment 2024-03-13] [$500] Workspace – Unable to save edited WS name using Enter key in newly created WS. #36923

[HOLD for payment 2024-03-13] [$500] Workspace – Unable to save edited WS name using Enter key in newly created WS. #36923

Comments

lanitochka17 commented Feb 20, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Feb 20, 2024

Krishna2323 commented Feb 20, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

Result

kadiealexander commented Feb 21, 2024

mvtglobally commented Feb 21, 2024 • edited Loading

kadiealexander commented Feb 21, 2024

kadiealexander commented Feb 21, 2024

melvin-bot bot commented Feb 21, 2024

melvin-bot bot commented Feb 21, 2024

bernhardoj commented Feb 21, 2024

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

getusha commented Feb 21, 2024

melvin-bot bot commented Feb 21, 2024

melvin-bot bot commented Feb 21, 2024

melvin-bot bot commented Feb 21, 2024

bernhardoj commented Feb 22, 2024

melvin-bot bot commented Mar 6, 2024

melvin-bot bot commented Mar 6, 2024

melvin-bot bot commented Mar 6, 2024 • edited by kadiealexander Loading

kadiealexander commented Mar 12, 2024

getusha commented Mar 12, 2024

kadiealexander commented Mar 13, 2024 • edited Loading

lanitochka17 commented Feb 20, 2024 •

edited by melvin-bot bot

Loading

Krishna2323 commented Feb 20, 2024 •

edited

Loading

mvtglobally commented Feb 21, 2024 •

edited

Loading

melvin-bot bot commented Mar 6, 2024 •

edited by kadiealexander

Loading

kadiealexander commented Mar 13, 2024 •

edited

Loading