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.

Sign up for GitHub

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

Jump to bottom

show profile errors #5525

Merged
merged 7 commits into from
Oct 22, 2021
Merged

show profile errors #5525

merged 7 commits into from
Oct 22, 2021

Conversation

thesahindia
Copy link
Member

@thesahindia thesahindia commented Sep 25, 2021 •
edited
Loading

Details

Proposal: #5091 (comment)
Approval: #5091 (comment)
After this PR is merged, we will be showing error messages when longer names (50+ chars) are being saved.
And profile updation will now wait for the API call to be completed to show the correct msg to user.

Fixed Issues

$ #5091

Tests

  1. Navigate to settings > Profile
  2. Change the first or last name to something over 50 characters
  3. Save the changes
  4. error message be shown
  5. Refresh the page or close the modal
  6. Name will remain unchanged if there is an error.

QA Steps

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

web.mp4

Mobile Web

mobile.web.mp4

Desktop

iOS

Android

Screenrecorder-2021-09-25-19-23-25-627.mp4

@thesahindia thesahindia marked this pull request as ready for review September 25, 2021 14:23
@thesahindia thesahindia requested a review from a team as a code owner September 25, 2021 14:23
@MelvinBot MelvinBot requested review from Beamanator and removed request for a team September 25, 2021 14:23
thesahindia and others added 2 commits September 30, 2021 13:59
@thesahindia @Beamanator
Update src/libs/actions/PersonalDetails.js
4f69ebc 
Co-authored-by: Alex Beaman <dabeamanator@gmail.com>
@thesahindia
remove whitespace
b872e86
@Beamanator Beamanator self-requested a review September 30, 2021 12:08
Beamanator
Beamanator approved these changes Sep 30, 2021
View reviewed changes
Copy link
Contributor

@Beamanator Beamanator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks for dealing with my change requests :)

@Beamanator Beamanator mentioned this pull request Oct 20, 2021
Closed
Beamanator
Beamanator requested changes Oct 20, 2021
View reviewed changes
src/languages/en.js Outdated Show resolved Hide resolved
Beamanator
Beamanator approved these changes Oct 22, 2021
View reviewed changes
Copy link
Contributor

@Beamanator Beamanator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Nice work 👍

@Beamanator Beamanator merged commit 0f21667 into Expensify:main Oct 22, 2021
@OSBotify
Copy link
Contributor

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

@OSBotify OSBotify mentioned this pull request Oct 23, 2021
Closed
@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @Beamanator in version: 1.1.8-10 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.10-2 🚀

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

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

@Beamanator Beamanator Beamanator approved these changes

Assignees

@thesahindia thesahindia

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@thesahindia @OSBotify @Beamanator