Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$500] PR 27825 RM - Save button should remain disabled until something is entered in the input field. #27998

Closed
1 of 6 tasks
izarutskaya opened this issue Sep 22, 2023 · 27 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@izarutskaya
Copy link

izarutskaya commented Sep 22, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Launch App
  2. Open Request Money > Distance
  3. Open the Start waypoint
  4. Press Save without selecting any address.

Expected Result:

If we don't enter anything atleast "Save" button should be disabled.

Actual Result:

App returns to the previous page when tap on the save button. Save button should remain disabled until something is entered in the input field.

Workaround:

Unknown

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: v1.3.72-6

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

Notes/Photos/Videos: Any additional supporting documentation

Bug6208917_27825_desktop.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause-Internal Team

Slack conversation: @

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01153f7357f12f779a
  • Upwork Job ID: 1706211261961732096
  • Last Price Increase: 2023-09-25
@izarutskaya izarutskaya added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 22, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 22, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Sep 22, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@izarutskaya
Copy link
Author

Found when validating PR : #27825

@DylanDylann
Copy link
Contributor

DylanDylann commented Sep 22, 2023

Proposal

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

Save button should remain disabled until something is entered in the input field

What is the root cause of that problem?

We don't have any validation in case the waypointValue is empty

const validate = (values) => {

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

We can add validation like this

 if (waypointValue === '') {
            ErrorUtils.addErrorMessage(errors, `waypoint${waypointIndex}`, 'common.error.fieldRequired');
        }

If we want to apply this validation only for start point and finish point we can add condition like this

 if (waypointValue === '') {
            ErrorUtils.addErrorMessage(errors, `waypoint${waypointIndex}`, 'common.error.fieldRequired');
        }

Result

Screenshot 2023-09-28 at 23 24 37

@rajsixthblock
Copy link

rajsixthblock commented Sep 22, 2023

### Proposal

Please re-state the problem that we are trying to solve in this issue.
Save button should be disabled until field is empty

What is the root cause of that problem?
Save button by default showing Active all the time, we need to disable the button

What changes do you think we should make in order to solve the problem?
We have to check input field is empty or not . we need to disable the save button by passing the value of input field as props from Form component to FormAlertWithSubmitButton component, using that value we need to write condition to disable or enable the button in FormAlertWithSubmitButton component

@melvin-bot melvin-bot bot added the Overdue label Sep 25, 2023
@greg-schroeder greg-schroeder added the External Added to denote the issue can be worked on by a contributor label Sep 25, 2023
@melvin-bot melvin-bot bot changed the title PR 27825 RM - Save button should remain disabled until something is entered in the input field. [$500] PR 27825 RM - Save button should remain disabled until something is entered in the input field. Sep 25, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 25, 2023

Job added to Upwork: https://www.upwork.com/jobs/~01153f7357f12f779a

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 25, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 25, 2023

Triggered auto assignment to @johncschuster (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@greg-schroeder
Copy link
Contributor

Applying External

@melvin-bot melvin-bot bot removed the Overdue label Sep 25, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 25, 2023

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

@RobertAndreiGit
Copy link

Proposal

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

Save button should remain disabled until something is entered in the input field.

What is the root cause of that problem?

No validation is made on the input field.

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

Have the disabled status of the save button updated based on the input field, having it disabled when it's empty.

@melvin-bot
Copy link

melvin-bot bot commented Sep 25, 2023

📣 @RobertAndreiGit! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@RobertAndreiGit
Copy link

Contributor details
Your Expensify account email: robert_amarandei@yahoo.com
Upwork Profile Link: https://www.upwork.com/freelancers/robertsky

@melvin-bot
Copy link

melvin-bot bot commented Sep 25, 2023

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@iamuddeshya
Copy link

Proposal

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

Clicking Save on empty input returns to previous page

What is the root cause of that problem?

The waypoint is intentionally allowed to be blank in the code. Not sure if this was done intentionally, hence will request someone to confirm.

if (waypointValue === '') {
Transaction.removeWaypoint(transactionID, waypointIndex);
}

and the validate function has no code for validating the empty waypoint value.

The above are the RCE for this problem.

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

Just add a condition waypointValue === ' ' here as well

if (!isOffline && waypointValue !== '' && waypointAddress !== waypointValue) {
ErrorUtils.addErrorMessage(errors, `waypoint${waypointIndex}`, 'distance.errors.selectSuggestedAddress');
}

However, this will not disable the Save button, but if the field is empty it will show an error to select an address.
I think this must be acceptable because this is how it is happening everywhere in the app. I have not seen the button's to be disabled, but instead an error message be shown.

Still if need to disable the button if input empty, we can pass a prop down the component and add a condition to disable the button.
BUT, as soon as a value is typed we need to enable it as well. Hence we need to have a 2 way data binding so that as soon as the input character receives a character, the input must automatically be enabled. Unlike the solutions that are mentioned above, in those cases the Save button will not automatically become enable after the text is entered.

What alternative solution do you find?

As stated in another proposal, we can handle this condition generically throughout the app by handling it within the form component it self.

@Mounika-Donga
Copy link

Using hooks we can able to solve the issue easily in React js.

useState you have to use
const [state, updatedState] = useState(true);

If we create a button, it is enable by default. For disabling the button, we have to pass disabled={state}.

Based on your Query, we have to call a arrow function for the on-change function of input field, here event will trigger, when we enter something. So, here we have to write a if condition inside of the function was, if event has some length, only then updatedState should be set as false. Then the state value will hits to button disabled mode. Initially the state was set as true, but if we enter something in input field, then the event has some length, then the updatedState is setting as false in input field on-change function. So, based on your requirement, the save button, will only be enable, when we enter something on input field, until it will be disable mode only.

Hope you understand and the solution I have wrote will work definitely.

Thank You.......

@melvin-bot
Copy link

melvin-bot bot commented Sep 25, 2023

📣 @Mounika-Donga! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@carlitose
Copy link

With the implementation of React hooks, we can streamline the process of managing form inputs and button states. This solution is designed to resolve the issue seamlessly.

Here's a brief overview:

  1. Use State Hook: We will use the useState hook to manage the state of the button. The initial state is set to true indicating that the button is disabled by default.
const [state, updateState] = useState(true);
  1. Button State: The button's 'disabled' attribute will be controlled by the state. By default, it will be disabled.
<button disabled={state}>Submit</button>
  1. Input Field & Event Handling: An arrow function will be called on the 'onChange' event of the input field. This event triggers whenever a user enters something into the field.
<input type="text" onChange={handleChange} />

Inside this function, we'll include a conditional statement. If the input field has some content (i.e., its length is not zero), updateState will be set to false, enabling the button.

const handleChange = (e) => {
  if (e.target.value.length > 0) {
    updateState(false);
  }
};

With this setup, the 'Submit' button will only be enabled when the input field contains some text. If the field is empty, the button will remain disabled.

I hope this explanation is clear. I'm confident that this solution will effectively address the issue at hand.

Thank you for considering this proposal.

Contributor details
Your Expensify account email: carlog.sergi@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01b0d12422bbdbf2d3

@melvin-bot
Copy link

melvin-bot bot commented Sep 25, 2023

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@zmaqutu
Copy link

zmaqutu commented Sep 25, 2023

Proposal

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

The issue at hand pertains to the absence of error handling for empty or whitespace-only waypoint values.

What is the root cause of that problem?

The root cause of this problem is the absence of whitespace validation when assessing the waypointValue during the validation process. This deficiency is observed in the code at line 107 of the WaypointEditor

if (isOffline && waypointValue !== '' && !ValidationUtils.isValidAddress(waypointValue)) {
ErrorUtils.addErrorMessage(errors, `waypoint${waypointIndex}`, 'bankAccount.error.address');
}

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

As specified in Forms.md, form submit buttons shall not be disabled. So disabling the button is out of the question.

Screenshot from 2023-09-25 13-04-58

I suggest we instead add || whiteSpaceRegex.test(waypointValue)as an additional condition in the if statement on line 107. This alteration will guarantee that an error is triggered when a user attempts to save an empty address.

const validate = (values) => {
        const errors = {};
        const whiteSpaceRegex = /^\s*$/;
        const waypointValue = values[`waypoint${waypointIndex}`] || '';
        if (isOffline && waypointValue !== '' && !ValidationUtils.isValidAddress(waypointValue)) {
            ErrorUtils.addErrorMessage(errors, `waypoint${waypointIndex}`, 'bankAccount.error.address');
        }

        // If the user is online and they are trying to save a value without using the autocomplete, or are trying to save an empty address, show an error message instructing them to use a selected address instead.
        // That enables us to save the address with coordinates when it is selected
        if ((!isOffline && waypointValue !== '' && waypointAddress !== waypointValue) || whiteSpaceRegex.test(waypointValue)) {
            ErrorUtils.addErrorMessage(errors, `waypoint${waypointIndex}`, 'distance.errors.selectSuggestedAddress');
        }

        return errors;
    };

What alternative solutions did you explore? (Optional)

NA

@melvin-bot
Copy link

melvin-bot bot commented Sep 25, 2023

📣 @zmaqutu! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@zmaqutu
Copy link

zmaqutu commented Sep 25, 2023

Contributor details
Your Expensify account email: zmaqutu@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01a04598e049270d15

@melvin-bot
Copy link

melvin-bot bot commented Sep 25, 2023

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@johncschuster
Copy link
Contributor

Looks like you've got this handled, @greg-schroeder. Unassigning myself!

@johncschuster johncschuster removed their assignment Sep 26, 2023
@zmaqutu
Copy link

zmaqutu commented Sep 26, 2023

Fix:

4824d7f07fb24f73a98c940172bc2b69.mp4

@jjcoffee
Copy link
Contributor

I don't think the expected result is correct as we typically avoid disabling the submit button unless an API request is pending (see here).

I think we're possibly expecting an error, but only in the case of there being only a start and end point (i.e. no intermediate stops), see this comment on the original PR.

So in this video the behaviour is correct (no error), as we want to allow an empty address field as a way of deleting waypoints.

Screen.Recording.2023-08-16.at.11.26.34.AM.mov

cc @thienlnam since you worked on the original waypoints PR, do you know/remember if it's expected to have an error if no address is entered (whilst online) and the save button is pressed?

@DylanDylann
Copy link
Contributor

updated proposal

@thienlnam
Copy link
Contributor

I'm closing this as it isn't a bug - you can save an empty waypoint to remove waypoints

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests