Skip to content

Clicking on 'fix the errors' button first input border is cut off at the top #12942

Closed

Description

@kavimuru

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. Go to URL https://staging.new.expensify.com/
  2. Login with Expensifail account
  3. Go to Settings => Payments => Add a debit card
  4. Without entering any data, click "Save"
  5. Click on 'fix the errors' button
  6. System should focus on the first input

Expected Result:

The first input should NOT be cut off from above

Actual Result:

The first input is cut off from above

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • IOS
  • Android

Version Number: 1.2.30-0
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

https://user-images.githubusercontent.com/43996225/203416843-0f7ba9d7-ad2e-4f50-aab2-2aa895ce5b47.mp4

Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation:

View all open jobs on GitHub

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Nov 22, 2022
  2. melvin-bot commented on Nov 22, 2022

    @melvin-bot

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

  3. MitchExpensify commented on Nov 22, 2022

    @MitchExpensify
    Contributor

    https://stackoverflow.com/c/expensify/questions/14418

    • The "bug" is actually a bug - Yes, clicking "Fix the errors" should take you to the first error input.
    • The bug is not a duplicate report of an existing GH (close the GH and add any novel details to the original GH instead) - This is super similar to this issue although it refers to a different pages [HOLD for payment 2022-11-29] [$250] First field is not focused after clicking the fix the errors link on Connect Manually page reported by @parasharrajat  #12400. I wonder if we should make sure that the "fix the errors" button jumps to first input with error in all cases - Is this a duplicate in your opinion @ctkochan22 ?
    • The bug is reproducible, following the reproduction steps:
    • If you’re unable to reproduce the bug:
    • Add the Needs reproduction label.
    • Comment on the issue outlining the steps you took to try to reproduce the bug, your results and tag the issue reporter and the Applause QA member who created the issue. Ask them to clarify reproduction steps and/or to check the reproduction steps again. Close issue.
  4. ctkochan22 commented on Nov 22, 2022

    @ctkochan22

    Hmmm its similar but I guess technically different bugs? This PR made changes to the form component. I'm not sure if this was an issue before or after this PR because we only tested it on forms that did not require a scroll

  5. self-assigned this
    on Nov 23, 2022
  6. Gonals commented on Nov 23, 2022

    @Gonals
    Contributor

    Taking a look!

  7. luacmartins commented on Nov 23, 2022

    @luacmartins
    Contributor

    IMO this is ok and a known drawback of using the focus() method to bring the input into view, instead of a much more complex implementation of scrolling to the input position with an offset. I don't feel strongly about it though and could entertain solutions for scrolling up and adding an offset.

  8. allroundexperts commented on Nov 26, 2022

    @allroundexperts
    Contributor

    Proposal

    As mentioned by @luacmartins, this is a known drawback of using focus method. We can therefore, scroll to the input position - an offset of 10.

    diff --git a/src/components/Form.js b/src/components/Form.js
    index 4734381bc..afdf1a06f 100644
    --- a/src/components/Form.js
    +++ b/src/components/Form.js
    @@ -1,6 +1,6 @@
     import lodashGet from 'lodash/get';
     import React from 'react';
    -import {ScrollView, View} from 'react-native';
    +import {ScrollView, View, findNodeHandle} from 'react-native';
     import PropTypes from 'prop-types';
     import _ from 'underscore';
     import {withOnyx} from 'react-native-onyx';
    @@ -78,8 +78,8 @@ class Form extends React.Component {
             };
     
             this.inputRefs = {};
    +        this.scrollViewRef = null;
             this.touchedInputs = {};
    -
             this.setTouchedInput = this.setTouchedInput.bind(this);
             this.validate = this.validate.bind(this);
             this.submit = this.submit.bind(this);
    @@ -249,6 +249,7 @@ class Form extends React.Component {
                         style={[styles.w100, styles.flex1]}
                         contentContainerStyle={styles.flexGrow1}
                         keyboardShouldPersistTaps="handled"
    +                    ref={ref => this.scrollViewRef = ref}
                     >
                         <View style={[this.props.style]}>
                             {this.childrenWrapperWithProps(this.props.children)}
    @@ -263,6 +264,9 @@ class Form extends React.Component {
                                     const errors = !_.isEmpty(this.state.errors) ? this.state.errors : this.props.formState.errorFields;
                                     const focusKey = _.find(_.keys(this.inputRefs), key => _.keys(errors).includes(key));
                                     this.inputRefs[focusKey].focus();
    +                                this.inputRefs[focusKey].measureLayout(findNodeHandle(this.scrollViewRef), (x, y) => {
    +                                    this.scrollViewRef.scrollTo({x, y: y - 10, animated: true});
    +                                });
                                 }}
                                 containerStyles={[styles.mh0, styles.mt5]}
                                 enabledWhenOffline={this.props.enabledWhenOffline}
    

    Result

    Simulator.Screen.Recording.-.iPhone.13.-.2022-11-26.at.18.07.48.mp4
  9. Gonals commented on Nov 28, 2022

    @Gonals
    Contributor

    Hey people! Please check the linked PRs before working on proposals! I grabbed this last week and the PR has been around since wednesday or so 😬

  10. mvtglobally commented on Nov 30, 2022

    @mvtglobally

    Issue not reproducible during KI retests. (First week)

  11. melvin-bot commented on Dec 9, 2022

    @melvin-bot

    @Gonals, @MitchExpensify 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

  12. melvin-bot commented on Dec 13, 2022

    @melvin-bot

    @Gonals, @MitchExpensify 10 days overdue. I'm getting more depressed than Marvin.

  13. melvin-bot commented on Dec 15, 2022

    @melvin-bot

    @Gonals, @MitchExpensify 12 days overdue now... This issue's end is nigh!

  14. melvin-bot commented on Dec 19, 2022

    @melvin-bot

    This issue has not been updated in over 14 days. @Gonals, @MitchExpensify eroding to Weekly issue.

  15. MitchExpensify commented on Dec 30, 2022

    @MitchExpensify
    Contributor

    Seeing as this PR hit prod and is not reproducible this should be closed, right @Gonals?

  16. Gonals commented on Dec 30, 2022

    @Gonals
    Contributor

    Yep!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

BugSomething is broken. Auto assigns a BugZero manager.EngineeringImprovementItem broken or needs improvement.ReviewingHas a PR in reviewWeeklyKSv2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions