Solved closing of keyboard on changing text field on profile page #9441

liyamahendra · 2022-06-14T09:10:57Z

Details

Fixed Issues

$ #9234

Tests

Mobile web:

Click on Settings and select 'Profile'
Click on 'First name' text input field. Notice the keyboard pop-up.
Now click on 'Last name' text input field. Make sure you click the text box where the 'Last name' label is and not inside the empty background.4.
Verify that the keyboard is not dismissed.

Verify that no errors appear in the JS console

PR Review Checklist

Contributor (PR Author) Checklist

PR Reviewer Checklist

QA Steps

Below steps are specifically for testing on mobile web.

Click on Settings and select 'Profile'
Click on 'First name' text input field. Notice the keyboard pop-up.
Now click on 'Last name' text input field. Make sure you click the text box where the 'Last name' label is and not inside the empty background.4.
The keyboard shown from step 2 should remain visible and not hide/dismiss.

Verify that no errors appear in the JS console

Screenshots

Web

Mobile Web

Desktop

iOS

Android

github-actions · 2022-06-14T09:11:20Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

liyamahendra · 2022-06-14T10:01:36Z

I have read the CLA Document and I hereby sign the CLA

rushatgabhane · 2022-06-14T14:13:01Z

@liyamahendra can you please add a new comment - "recheck" to trigger the CLA bot

liyamahendra · 2022-06-14T15:35:24Z

recheck

rushatgabhane

@liyamahendra

There are a few bug -

Fix error message is showing up on editing inputs
The checkbox "Set timezone automatically" isn't toggling
Clicking save button after making changes doesn't seem to do anything

Any ideas on how we can fix them?

Screen.Recording.2022-06-15.at.7.07.01.PM.mov

liyamahendra · 2022-06-15T16:53:02Z

Any ideas on how we can fix them?

@rushatgabhane I can definitely look into them. Seeing your shared recording, I couldn't infer why would the error message show-up at bottom. Plus, if there is an error in the Input Fields, the error message is shown below it.

Mind sharing the steps how can I reproduce what you saw / happened with you?

rushatgabhane · 2022-06-15T16:55:48Z

Mind sharing the steps how can I reproduce what you saw / happened with you

Click first name TextInput and modify it.
Click anywhere outside the TextInput.

I hope this helps!

liyamahendra · 2022-06-16T10:24:11Z

@rushatgabhane Thank you - the steps did help!

I've made the changes. Attached is the screencast after the changes.

update.mp4

src/pages/settings/Profile/ProfilePage.js

rushatgabhane · 2022-06-23T16:15:09Z

left a note here #9234 (comment)

luacmartins · 2022-06-23T16:40:49Z

@rushatgabhane let me know once you are done reviewing this since I wanna check the Form refactor portion of this PR.

src/CONST.js

src/pages/settings/Profile/ProfilePage.js

luacmartins · 2022-08-31T07:15:41Z

@liyamahendra this is more dependant on you than us! I feel like the delay has been caused by the code not following our guidelines and several bugs being caught during review. The fastest way to get PRs merged is to read our guidelines and make sure that your code is following it, as well as testing your changes well in all platforms before making them available for review - this reduces the amount of back and forth!

luacmartins · 2022-08-31T07:17:03Z

@liyamahendra we have conflicts again, can you please address those? @rushatgabhane do you still have any pending comments here?

rushatgabhane · 2022-08-31T19:29:29Z

Reviewing now.

src/pages/settings/Profile/ProfilePage.js

rushatgabhane

Click on the avatar picker and upload an avatar. Verify that no loading indicator appears, and that your avatar updates immediately after you click "Save" (the "Save" button in the cropping modal). Hard refresh () the page, and verify that the new avatar is persisted.

@liyamahendra In PR #10235, the above feature was added ^

But our PR is breaking it.

rushatgabhane · 2022-08-31T20:55:08Z

@luacmartins just noting that the submit button doesn't float above keyboard.

But this is an issue related to Form.js We should (if we want to) fix it in another issue by add a prop which would wrap the Button with FixedFooter.

Screen.Recording.2022-09-01.at.1.58.16.AM.mov

luacmartins · 2022-09-05T09:57:45Z

@liyamahendra I think the checks will pass once @rushatgabhane fills his C+ reviewer checklist. @rushatgabhane will you have some time to review this again today?

rushatgabhane

@liyamahendra thanks for your patience and fixing everything.

Requesting a change to use loadashGet (to prevent app crash)

src/pages/settings/Profile/ProfilePage.js

rushatgabhane

@luacmartins LGTM! 🎉

PR Reviewer Checklist

I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- iOS / native
- Android / native
- iOS / Safari
- Android / Chrome
- MacOS / Chrome
- MacOS / Desktop
I verified there are no console errors (if there’s a console error not related to the PR, report it or open an issue for it to be fixed)
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained “why” the code was doing something instead of only explaining “what” the code was doing.
- I verified any copy / text shown in the product was added in all src/languages/* files
- I verified any copy / text that was added to the app is correct English and approved by marketing by tagging the marketing team on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named “index.js”. All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- Any functional components have the displayName property
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose and it is broken down into smaller components in order to separate concerns and functions
If a new CSS style is added I verified that:
- A similar style doesn’t already exist
- The style can’t be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

luacmartins

@liyamahendra we are really close! Left comments to remove some empty lines and some constants that were defined twice (probably due to a bad conflict resolution). Let's get this addressed and merged today!

src/CONST.js

src/pages/settings/Profile/ProfilePage.js

luacmartins · 2022-09-06T08:41:09Z

Gonna merge, since @rushatgabhane had already approved this as well! Thanks for your patience @liyamahendra!

OSBotify · 2022-09-06T08:45:40Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

liyamahendra · 2022-09-06T17:45:44Z

@luacmartins With regards to the bounty you mentioned, how will that happen? Do I've to do anything? Please advise.

luacmartins · 2022-09-07T08:30:32Z

@liyamahendra the process is described here. I'm a bit confused by this comment though, did you already get this payment?

liyamahendra · 2022-09-07T08:45:02Z

Yes, I received the original payment of $250. Upwork auto approves the milestone payment after 14 days of submission of work. Thank you for sharing the link to the process.

OSBotify · 2022-09-08T01:21:51Z

🚀 Deployed to staging by @luacmartins in version: 1.1.98-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

Beamanator · 2022-10-26T12:21:39Z

src/pages/settings/Profile/ProfilePage.js

-                <ScrollView style={styles.flex1} contentContainerStyle={styles.p5}>
+                <Form
+                    style={[styles.flexGrow1, styles.ph5]}
+                    formID={CONST.PROFILE_SETTINGS_FORM}


FYI now'days we put the form IDs here:

App/src/ONYXKEYS.js

Lines 171 to 176 in ff5affd

FORMS: {

ADD_DEBIT_CARD_FORM: 'addDebitCardForm',

REQUEST_CALL_FORM: 'requestCallForm',

REIMBURSEMENT_ACCOUNT_FORM: 'reimbursementAccount',

WORKSPACE_SETTINGS_FORM: 'workspaceSettingsForm',

},

fixed closing of keyboard on changing text field on profile page

8e9b60d

liyamahendra requested a review from a team as a code owner June 14, 2022 09:10

melvin-bot bot requested review from marcochavezf and rushatgabhane and removed request for a team June 14, 2022 09:11

liyamahendra mentioned this pull request Jun 14, 2022

[HOLD for payment 2022-09-16] [$250] Changing input field on Mobile App closes the keyboard - reported by @Tushu17 #9234

Closed

rushatgabhane suggested changes Jun 15, 2022

View reviewed changes

liyamahendra added 2 commits June 16, 2022 14:52

Solved the incorrect display of error at bottom

54e0294

Solved automatic timezone checkbox not being clickable

26d331a

rushatgabhane reviewed Jun 16, 2022

View reviewed changes

src/pages/settings/Profile/ProfilePage.js Outdated Show resolved Hide resolved

rushatgabhane suggested changes Jun 17, 2022

View reviewed changes

src/pages/settings/Profile/ProfilePage.js Outdated Show resolved Hide resolved

src/pages/settings/Profile/ProfilePage.js Outdated Show resolved Hide resolved

Making CheckboxWithLabel compatible with the Form

060f307

rushatgabhane reviewed Jun 17, 2022

View reviewed changes

src/pages/settings/Profile/ProfilePage.js Outdated Show resolved Hide resolved

liyamahendra added 5 commits June 17, 2022 23:18

Renamed Set Automatic Timezone Checkbox inputID

d04ecac

Moved the character limit value to CONST

7d07035

refactored doesFailCharacterLimit function and ProfilePage validation

c4e1cf3

Refactoring ProfilePage validation

e844388

Fixed lint error

651efb7

luacmartins self-requested a review June 23, 2022 16:39

rushatgabhane suggested changes Jun 23, 2022

View reviewed changes

added JSDoc for validate method

885ef63

resolved conflicts

963d242

rushatgabhane reviewed Aug 31, 2022

View reviewed changes

src/pages/settings/Profile/ProfilePage.js Outdated Show resolved Hide resolved

rushatgabhane suggested changes Aug 31, 2022

View reviewed changes

src/pages/settings/Profile/ProfilePage.js Outdated Show resolved Hide resolved

src/pages/settings/Profile/ProfilePage.js Outdated Show resolved Hide resolved

src/pages/settings/Profile/ProfilePage.js Outdated Show resolved Hide resolved

rushatgabhane suggested changes Aug 31, 2022

View reviewed changes

liyamahendra added 3 commits September 3, 2022 12:06

Solved profile avatar update crash

447cd8a

Trigger checks again

a9c2cff

Trigger checks again

36061f6

rushatgabhane suggested changes Sep 5, 2022

View reviewed changes

src/pages/settings/Profile/ProfilePage.js Outdated Show resolved Hide resolved

src/pages/settings/Profile/ProfilePage.js Outdated Show resolved Hide resolved

Using lodashGet for firstName and lastName

9686c11

rushatgabhane previously approved these changes Sep 5, 2022

View reviewed changes

luacmartins requested changes Sep 6, 2022

View reviewed changes

src/CONST.js Outdated Show resolved Hide resolved

src/pages/settings/Profile/ProfilePage.js Outdated Show resolved Hide resolved

src/pages/settings/Profile/ProfilePage.js Outdated Show resolved Hide resolved

src/pages/settings/Profile/ProfilePage.js Show resolved Hide resolved

Removed unused constants

a796a5b

liyamahendra dismissed rushatgabhane’s stale review via a796a5b September 6, 2022 08:33

removed empty lines

d035720

luacmartins approved these changes Sep 6, 2022

View reviewed changes

luacmartins merged commit 1708742 into Expensify:main Sep 6, 2022

liyamahendra deleted the 9234 branch September 6, 2022 08:41

OSBotify mentioned this pull request Sep 8, 2022

Deploy Checklist: New Expensify 2022-09-08 #10883

Closed

25 tasks

Beamanator reviewed Oct 26, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Solved closing of keyboard on changing text field on profile page #9441

Solved closing of keyboard on changing text field on profile page #9441

liyamahendra commented Jun 14, 2022 •

edited by luacmartins

Loading

github-actions bot commented Jun 14, 2022 •

edited

Loading

liyamahendra commented Jun 14, 2022

rushatgabhane commented Jun 14, 2022

liyamahendra commented Jun 14, 2022

rushatgabhane left a comment •

edited

Loading

liyamahendra commented Jun 15, 2022 •

edited

Loading

rushatgabhane commented Jun 15, 2022 •

edited

Loading

liyamahendra commented Jun 16, 2022

rushatgabhane commented Jun 23, 2022

luacmartins commented Jun 23, 2022

luacmartins commented Aug 31, 2022

luacmartins commented Aug 31, 2022

rushatgabhane commented Aug 31, 2022 •

edited

Loading

rushatgabhane left a comment •

edited

Loading

rushatgabhane commented Aug 31, 2022

luacmartins commented Sep 5, 2022

rushatgabhane left a comment

rushatgabhane left a comment

luacmartins left a comment

luacmartins commented Sep 6, 2022

OSBotify commented Sep 6, 2022

liyamahendra commented Sep 6, 2022

luacmartins commented Sep 7, 2022 •

edited

Loading

liyamahendra commented Sep 7, 2022

OSBotify commented Sep 8, 2022

Beamanator Oct 26, 2022

	FORMS: {
	ADD_DEBIT_CARD_FORM: 'addDebitCardForm',
	REQUEST_CALL_FORM: 'requestCallForm',
	REIMBURSEMENT_ACCOUNT_FORM: 'reimbursementAccount',
	WORKSPACE_SETTINGS_FORM: 'workspaceSettingsForm',
	},

Solved closing of keyboard on changing text field on profile page #9441

Solved closing of keyboard on changing text field on profile page #9441

Conversation

liyamahendra commented Jun 14, 2022 • edited by luacmartins Loading

Details

Fixed Issues

Tests

Mobile web:

PR Review Checklist

Contributor (PR Author) Checklist

PR Reviewer Checklist

QA Steps

Screenshots

Web

Mobile Web

Desktop

iOS

Android

github-actions bot commented Jun 14, 2022 • edited Loading

liyamahendra commented Jun 14, 2022

rushatgabhane commented Jun 14, 2022

liyamahendra commented Jun 14, 2022

rushatgabhane left a comment • edited Loading

Choose a reason for hiding this comment

liyamahendra commented Jun 15, 2022 • edited Loading

rushatgabhane commented Jun 15, 2022 • edited Loading

liyamahendra commented Jun 16, 2022

rushatgabhane commented Jun 23, 2022

luacmartins commented Jun 23, 2022

luacmartins commented Aug 31, 2022

luacmartins commented Aug 31, 2022

rushatgabhane commented Aug 31, 2022 • edited Loading

rushatgabhane left a comment • edited Loading

Choose a reason for hiding this comment

rushatgabhane commented Aug 31, 2022

luacmartins commented Sep 5, 2022

rushatgabhane left a comment

Choose a reason for hiding this comment

rushatgabhane left a comment

Choose a reason for hiding this comment

luacmartins left a comment

Choose a reason for hiding this comment

luacmartins commented Sep 6, 2022

OSBotify commented Sep 6, 2022

liyamahendra commented Sep 6, 2022

luacmartins commented Sep 7, 2022 • edited Loading

liyamahendra commented Sep 7, 2022

OSBotify commented Sep 8, 2022

Beamanator Oct 26, 2022

Choose a reason for hiding this comment

liyamahendra commented Jun 14, 2022 •

edited by luacmartins

Loading

github-actions bot commented Jun 14, 2022 •

edited

Loading

rushatgabhane left a comment •

edited

Loading

liyamahendra commented Jun 15, 2022 •

edited

Loading

rushatgabhane commented Jun 15, 2022 •

edited

Loading

rushatgabhane commented Aug 31, 2022 •

edited

Loading

rushatgabhane left a comment •

edited

Loading

luacmartins commented Sep 7, 2022 •

edited

Loading