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.56k stars 2.9k forks source link

[$500] Status-Set clear status & able to save status without selecting emoji or entering message #35647

Closed izarutskaya closed 9 months ago

izarutskaya commented 9 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: 1.4.35 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause-Internal Team Slack conversation:

Action Performed:

  1. Go to https://staging.new.expensify.com/
  2. Tap profile icon
  3. Tap profile --- Status
  4. Tap clear status
  5. Tap custom
  6. Select date, set time and save it.
  7. Now without selecting any emoji and entering status message, tap save

Expected Result:

Without setting status emoji or message, tapping save must show error to select status emoji before saving it.

User sets clear status & must not be able to save status without selecting emoji or entering message.

Actual Result:

Without setting status emoji or message, tapping save not showing any error to select status emoji before saving it.

User sets clear status & able to save status without selecting emoji or entering message.

When there is no status message, allowing user to save clear status and empty status message has no point of use.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

https://github.com/Expensify/App/assets/115492554/4ceca9b6-5b9c-4fe0-9ba0-9eebf97fc382

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0161ec5a5613f04872
  • Upwork Job ID: 1753358436702269440
  • Last Price Increase: 2024-02-02
melvin-bot[bot] commented 9 months ago

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

melvin-bot[bot] commented 9 months ago

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

melvin-bot[bot] commented 9 months ago

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

paultsimura commented 9 months ago

Proposal

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

Users can save an empty status with custom clear after.

What is the root cause of that problem?

The issue happens because we do not check the text & emoji to not be empty neither during validation nor during saving the value.

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

Even though the expected behavior says we should show an error in this case, I think it's a better UX to allow the user to clear their status by setting an empty value. We just should call clearStatus if emojiCode andstatusText are empty:

    const updateStatus = useCallback(
        ({emojiCode, statusText}) => {
            if (!emojiCode && _.isEmpty(statusText)) {
                clearStatus();
                return;
            }
...

https://github.com/Expensify/App/blob/d5eba656251269b238797817df3cf84800b0039a/src/pages/settings/Profile/CustomStatus/StatusPage.js#L74-L94

What alternative solutions did you explore? (Optional)

If we want to show a validation error instead, we should add handling for a case when neither emoji nor text is selected/entered.

We should show the validation error in this case:

    const validateForm = useCallback(({emojiCode, statusText}) => {
        const errors = {};
        if (!emojiCode && _.isEmpty(statusText)) {
            ErrorUtils.addErrorMessage(errors, INPUT_IDS.STATUS_TEXT, 'statusPage.statusRequired');
        }
        if (brickRoadIndicator) {
            errors.clearAfter = '';
        }
        return errors;
    }, [brickRoadIndicator]);

https://github.com/Expensify/App/blob/d5eba656251269b238797817df3cf84800b0039a/src/pages/settings/Profile/CustomStatus/StatusPage.js#L122-L127

hoangzinh commented 9 months ago

@paultsimura just wanna clarify the requirement here, I think we would like to not allow users to save clear status setting without selecting an emoji or message. It doesn't mean we don't allow users to clear their status by setting an empty value in all cases.

paultsimura commented 9 months ago

@hoangzinh how should this look UX-wise considering there is no such option as "do not set clear status setting" and the default value is automatically set to "Today"?

https://github.com/Expensify/App/blob/3ab4e6e12d9ff38e69493321f0d31f032481b373/src/pages/settings/Profile/CustomStatus/StatusPage.js#L111-L114

image

As the Today is also a value, we should show an error in this flow as well?

  1. The user has no status set
  2. Opens the "Status" page
  3. The default value of clearAfter is set to Today
  4. The User decides not to enter any value and clicks "Save"
  5. Show the error?

Won't it be a bit confusing for the user considering they've not entered any value and try to save the "blank" state?

hoangzinh commented 9 months ago

Thanks for raising it @paultsimura

What do you think about the above question https://github.com/Expensify/App/issues/35647#issuecomment-1925435610 @kadiealexander ?

kadiealexander commented 9 months ago

I actually don't think this is a bug! When you click "clear status" the status saves automatically:

https://github.com/Expensify/App/assets/59587260/ea6e72fb-0230-4c56-9358-9237c45d8085

And if you save a clear status, even with the clearAfter set to today it will just remain with no status after "today". Please reopen if you disagree!

paultsimura commented 9 months ago

@kadiealexander please re-read the steps of the issue. It's not about the "clear status" button. It's when you have cleared your status (or didn't have one), then set the custom time for "clear after" and click "save" — you'll be able to save an empty status with a defined "clear after" time, which is incorrect UX.

kadiealexander commented 9 months ago

Right! I still don't think this is an issue. Like I said in my comment above: if you save an empty status, even with the clearAfter set to a specific date it will just remain with no status after the end date. It's slightly weird UX, but it's not necessarily incorrect and it's not causing a major error or blocking the user from using Expensify, so I vote to keep it closed. If you strongly disagree, please feel free to bring the conversation to Slack for more thoughts.