[$250] Room - "Please enter a room name" error appears when switching to the Room tab

IuliiaHerets commented 2 months 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.19-9 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/4849474&group_by=cases:section_id&group_id=296763&group_order=asc Issue reported by: Applause Internal Team

Action Performed:

Open the app
Log in with a new Gmail account
Create a workspace
Navigate to the LHN
Navigate to FAB - Start chat
Tap on the "Room" tab

Expected Result:

The error should only appear when i tap the "Create room" button without entering any room name.

Actual Result:

"Please enter a room name" error appears when switching to the Room tab.

Workaround:

Unknown

Platforms:

[ ] Android: Native
[ ] Android: mWeb Chrome
[x] iOS: Native
[ ] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/0af238bd-f143-4fb6-b884-39bbc9010844

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~018c5e8d8993a6b3ba
Upwork Job ID: 1823441957241129725
Last Price Increase: 2024-08-13
Automatic offers:

ahmedGaber93 | Reviewer | 103622010
dominictb | Contributor | 103622012

Issue Owner

Current Issue Owner: @ahmedGaber93

melvin-bot[bot] commented 2 months ago

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

IuliiaHerets commented 2 months ago

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

IuliiaHerets commented 2 months ago

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

dominictb commented 2 months ago

Edited by proposal-police: This proposal was edited at 2024-08-13 15:02:15 UTC.

Proposal

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

"Please enter a room name" error appears when switching to the Room tab.

What is the root cause of that problem?

We have the logic to validate form when the input get blurred

https://github.com/Expensify/App/blob/c5feb89f9f02cfb303e86246408f40c4f5a23119/src/components/Form/FormProvider.tsx#L336

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

We should check if the page is not focused then do nothing

    const isFocusedRef = useRef(true);

    useFocusEffect(
        useCallback(() => {
            isFocusedRef.current = true;
            return () => {
                isFocusedRef.current = false;
            };
        }, []),
    );

...

if (shouldValidateOnBlur && isFocusedRef.current) {
                                onValidate(inputValues, !hasServerError);
                            }

What alternative solutions did you explore? (Optional)

We can consider to create the hook to detect isFocusedRef

nyomanjyotisa commented 2 months ago

Proposal

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

"Please enter a room name" error appears when switching to the Room tab

What is the root cause of that problem?

we don't set shouldValidateOnBlur here and its default value on FormProvider is true https://github.com/Expensify/App/blob/c5feb89f9f02cfb303e86246408f40c4f5a23119/src/pages/workspace/WorkspaceNewRoomPage.tsx#L270-L277

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

IMO we can set shouldValidateOnBlur to false here since it is the only required input text on start room chat

What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

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

Krishna2323 commented 2 months ago

Proposal

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

Room - "Please enter a room name" error appears when switching to the Room tab

Switching to Chat and going back to Room shows error.

What is the root cause of that problem?

The form isn't reseted when screen lost's focus.
This is an inconsistency because we also use tab navigator on start IOU request page and there if we switch tab the data and errors are cleared. See the video below.

https://github.com/user-attachments/assets/66a844a2-d574-488e-9993-8a834817a041

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

We can clear the form errors when screen focus is changed.

We can create a ref for FormProvider to get the resetForm function through useImperativeHandle
In WorkspaceNewRoomPage we need to call the formRef.current?.resetForm() when screen is unmounted. The same behaviour can also be seen on IOURequestStartPage, we clear the form values and errors when tab gets changed.

    useEffect(() => {
        const ref = formRef.current;
        if (isFocused) {
            return;
        }

        return () => ref?.resetForm({});
    }, [selectedTab, isFocused]);

We call also clear the selected participants and search term on chat page if needed.
What alternative solutions did you explore? (Optional)

dominictb commented 2 months ago

@ahmedGaber93 For more information

I tried to test the problem on iOS, it works well without any changes. onBlur is not triggered

https://github.com/user-attachments/assets/329a2ac8-5ed9-46fc-b6ba-ab06bf955b95

So we need to fix this bug on web. When users switch between tabs without any changes, they shouldn't see the error message

https://github.com/user-attachments/assets/5dbb8e20-e970-40ce-8ea1-05760c357a00

BTW, when users blur the input on room page, they will see the error message (like other page) -> it should preserve the error when users go to Chat and back to Room since they already see the error before -> There's no confusion

Krishna2323 commented 2 months ago

Proposal Updated

Updated RCA to point out the inconsistency we have between new chat page and IOU start page, both uses tab navigator bug have different behaviours.

dominictb commented 2 months ago

@ahmedGaber93 What do you think about my proposal? Thanks

ahmedGaber93 commented 2 months ago

Reviewing today

ahmedGaber93 commented 2 months ago

Thanks all for the proposals.

@dominictb's proposal add a solution to prevent onValidate only while switching the tab.

@nyomanjyotisa's proposal add a solution to prevent onValidate all the time.

@Krishna2323's proposal add a solution to reset the form after tab switched.

While all the proposals can fix the issue, but @dominictb's proposal is the only that can fix it without change the current UX.

@dominictb's proposal LGTM!

🎀 👀 🎀 C+ reviewed

melvin-bot[bot] commented 2 months ago

Current assignee @puneetlath is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

Krishna2323 commented 2 months ago

This is an inconsistency because we also use tab navigator on start IOU request page and there if we switch tab the data and errors are cleared. See the video below.

https://github.com/user-attachments/assets/66a844a2-d574-488e-9993-8a834817a041

@puneetlath, do you think we should clear the draft data when switching room tabs, just like we do when starting IOU request tabs? If not then you can go with the selected proposal.

ahmedGaber93 commented 2 months ago

This is an inconsistency because we also use tab navigator on start IOU request page and there if we switch tab the data and errors are cleared. See the video below.

I think the reset in IOU is for resetting the draft transaction in Onyx and not for design/UX purpose

puneetlath commented 2 months ago

I don't think we need to clear the input in this scenario. So @dominictb's proposal sounds good to me. But are there other similar forms on the site? And if so, would they need a similar solution?

dominictb commented 2 months ago

@puneetlath My solution will fix on FormProvider, so we just need to update one place.

melvin-bot[bot] commented 2 months ago

📣 @ahmedGaber93 🎉 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[bot] commented 2 months ago

📣 @dominictb 🎉 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 📖

ahmedGaber93 commented 1 month ago

Regression Test Proposal

Click on FAB icon > Start chat
Tap back and forth between on the "Room" and the "Chat" tab
Verify that: "Please enter a room name" error does not appear when switching the tab.

Do we agree 👍 or 👎

ahmedGaber93 commented 1 month ago

@puneetlath This issue is ready for payment, The PR was deployed to production for 7 days ago (2/9) base on here https://github.com/Expensify/App/pull/47908#issuecomment-2325378109

puneetlath commented 1 month ago

All paid. Thanks everyone!

Expensify / App