Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.12k stars 2.61k forks source link

[$150] Web - Workspace - Invite new members" field is not focused #44182

Open lanitochka17 opened 2 weeks ago

lanitochka17 commented 2 weeks ago

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.0-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): gibethlehem@gmail.com Issue reported by: Applause - Internal Team

Action Performed:

  1. Log in to an account
  2. Go to workspace(Or create one) > Members
  3. Click on the "Invite member" button at the top. Notice the field is focused
  4. On the Invite members page click on the Question mark button to go to the "Get assistance"
  5. Click back to return to the Invite Members page

Expected Result:

Invite members page field is focused

Actual Result:

The invite members page field is not focused

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/f1c864c8-e283-4738-ba75-83cf8967f78a

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01bda364df69d71d2a
  • Upwork Job ID: 1805292410395622118
  • Last Price Increase: 2024-06-24
  • Automatic offers:
    • tsa321 | Contributor | 102885241
Issue OwnerCurrent Issue Owner: @rushatgabhane
melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @adelekennedy (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 commented 2 weeks ago

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

lanitochka17 commented 2 weeks ago

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

Krishna2323 commented 2 weeks ago

Proposal

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

Web - Workspace - Invite new members" field is not focused

What is the root cause of that problem?

We are directly using useFocusEffect & useCallback which does not cover cases like isScreenTransitionEnded and due to this the input is not focused correctly. https://github.com/Expensify/App/blob/854a8a1a1f5348180217ad492cc773dc3615943f/src/components/SelectionList/BaseSelectionList.tsx#L525-L538

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

We should use inputCallbackRef from useAutoFocusInput which is designed to focus on the input on every focus on the screen.

    useEffect(() => {
        if (!textInputAutoFocus || !shouldShowTextInput) {
            return;
        }
        inputCallbackRef(innerTextInputRef.current);
    }, [textInputAutoFocus, inputCallbackRef, shouldShowTextInput]);

    const prevTextInputValue = usePrevious(textInputValue);
    const prevSelectedOptionsLength = usePrevious(flattenedSections.selectedOptions.length);

What alternative solutions did you explore? (Optional)

Use InteractionManager.runAfterInteractions in focusTextInput function. https://github.com/Expensify/App/blob/854a8a1a1f5348180217ad492cc773dc3615943f/src/components/SelectionList/BaseSelectionList.tsx#L516-L523

    /** Function to focus text input */
    const focusTextInput = useCallback(() => {
        if (!innerTextInputRef.current) {
            return;
        }

        InteractionManager.runAfterInteractions(() => {
            innerTextInputRef.current.focus();
        });
    }, []);
tsa321 commented 2 weeks ago

Proposal

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

Web - Workspace - Invite new members" field is not focused

What is the root cause of that problem?

Because of the newly added focusTrap and the Workspace_Invite screen is not included in SCREENS_WITH_AUTOFOCUS:

https://github.com/Expensify/App/blob/b021195b864211efbaaefe188226b2830b91b7d8/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx#L57-L61

When user returns to previous screen, focusTrap will refocus last focused element, return focus will return the ? button element and will get refocused. It is because, the ? button is last focused element before Get Assistance screen opened. The button element will be saved by focusTrap and will refocus when Get Assistace screen is closed. If we include the screen (SCREENS.WORKSPACE.INVITE) in SCREENS_WITH_AUTOFOCUS, the setReturnFocus will return false and won't refocus the ? button.

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

We should add the Workspace_Invite (SCREENS.WORKSPACE.INVITE) screen in SCREENS_WITH_AUTOFOCUS

alternatively, if we want to disable refocusing last active element in previous screen we could return false here..

dominictb commented 2 weeks ago

Proposal

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

What is the root cause of that problem?

and it is set to false if the search input is not focused:

https://github.com/Expensify/App/blob/425804aa005c6299ff0f59ff596764c429e19abd/src/components/SelectionList/BaseSelectionList.tsx#L456

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

adelekennedy commented 1 week ago

I can reproduce, and while this is a small bug I think it's worth it to fix

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

adelekennedy commented 1 week ago

@rushatgabhane already three proposals above to review

melvin-bot[bot] commented 1 week ago

Upwork job price has been updated to $150

rushatgabhane commented 1 week ago

@tsa321's proposal LGTM

πŸŽ€ πŸ‘€ πŸŽ€

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

πŸ“£ @tsa321 πŸŽ‰ 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 πŸ“–

tsa321 commented 1 week ago

@rushatgabhane PR is ready