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

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

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
Co-authored-by: Alex Beaman <dabeamanator@gmail.com>
@Beamanator Beamanator self-requested a review September 30, 2021 12:08
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 :)

src/languages/en.js Outdated Show resolved Hide resolved
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
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants