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

Create an alert growl component and add confirmation growl when saving changes in profile #2812

Closed
bfitzexpensify opened this issue May 11, 2021 · 14 comments · Fixed by #2944
Assignees

Comments

@bfitzexpensify
Copy link
Contributor

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


Expected Result:

Display a confirmation growl when saving a change in your profile — examples shown below

Actual Result:

Nothing happens

Action Performed:

  1. Make a change to your profile
  2. Confirm that no growl appears when saving

Platform:

All platforms

Version Number: All versions
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

image

image

Error growls:
image

Expensify/Expensify Issue URL:

View all open jobs on Upwork

@MelvinBot
Copy link

Triggered auto assignment to @thienlnam (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@webdev-dimuthu
Copy link

Hello.
I have built and run the app on my local, and I have checked the current issue.
I can fix the issue in 2 days.
What I am going to do are the followings.

  1. Validate the values that user input, and if it is invalid, show invalid pop-up notification.
  2. If the value of user input is valid, check the returned value of API.
    If the response of API is valid, show success pop-up notification.
    If the response of API is invalid, show invalid pop-up notification.
  3. Implement the pop-up notification component.
    Thanks.

Ekaterina.

@thienlnam
Copy link
Contributor

Hello @Ekaterina221,

Thanks for your proposal! I'm hoping for some more specifics with your proposal.

  1. Validate the values that user input, and if it is invalid, show invalid pop-up notification.

What is valid/invalid input? How are you going to show the invalid pop-up and where are you going to do this validation? (Files)

  1. Implement the pop-up notification component.

What is this component going to look like, and how are you going to achieve this? Are you going to use any libraries to achieve this? Example of how this component is going to be used, and how you'll trigger it to render at an invalid or successful notification.

@webdev-dimuthu
Copy link

I will explain to you more details.

  1. Validate the values that user input, and if it is invalid, show invalid pop-up notification.
    For this step, I am going to check the "First Name", "Last Name" and "Email Address".
    For example, if the user input "Ekaterina123" for the "Fiist Name", this will be invalid value.
    If the user input "testemail@mail" for the "Email Address", this will be invalid value.
    I am going to show invalid pop-up notification to the user.

  2. Implement the pop-up notification component.
    image
    I am going to show the pop-up notification like this.
    But the UI design is customized as you show me example.
    This is an external React Native package.
    When using the component, it can set various parameters to show success or fail.
    Thanks.

@thienlnam
Copy link
Contributor

Thanks for the explanations, I have a few more questions -

For this step, I am going to check the "First Name", "Last Name" and "Email Address".
For example, if the user input "Ekaterina123" for the "Fiist Name", this will be invalid value.
If the user input "testemail@mail" for the "Email Address", this will be invalid value.

How are you validating this input, are you planning on using regex?

What files are you making these changes in?

This is an external React Native package.

What package are you planning on using, and how will you use this package? (Will you wrap it inside a component?)
Does this package work on Web, Android, IOS, mobileWeb, and MacOS?

@thienlnam
Copy link
Contributor

Also, I don't think we need to use an external package here and should just make this component ourselves - can you outline how you would create this component manually?

@webdev-dimuthu
Copy link

For the validation, I am going to use RegExp.
And for the pop-up notification, I can create the component without using external package.
Thanks.

@kakajann
Copy link
Contributor

First off, there is a bug. When you save your profile without First name and Last name it doesn't update myPersonalDetails. Here's a video showing the bug.

Screen.Recording.2021-05-14.at.4.38.59.PM.mov

Also, showing a notification growl when required fields are empty is bad UX. (it confuses the user and force them to find what they miss in the form). Ideally, we should use something like this,
Screen Shot 2021-05-14 at 4 49 43 PM
and show the notification growl when only the profile is successfully saved.

Proposal

  1. Fix the bug appearing in the video above.
  2. Validate form fields using Formik and Yup (external packages to validate forms with RegExp built-in)
  3. Show a notification growl when updating user profile.

@shawnborton
Copy link
Contributor

shawnborton commented May 14, 2021

Also, showing a notification growl when required fields are empty is bad UX. (it confuses the user and force them to find what they miss in the form). Ideally, we should use something like this,

@kakajann I agree, we should fix the lack of form validation. However, I think we can do that as a separate issue from this one and focus on just creating the confirmation growl upon a successful save. I actually don't think those two fields are even required so that might not even be an issue right now.

@parasharrajat
Copy link
Member

As a suggestion, I think we should update the title to something like. Create an alert growl component and add confirmation growl when saving changes in profile.

@thienlnam thienlnam changed the title Add confirmation growl when saving changes in profile Create an alert growl component and add confirmation growl when saving changes in profile May 14, 2021
@thienlnam
Copy link
Contributor

@Ekaterina221

And for the pop-up notification, I can create the component without using external package.

I'm still hoping for an outline of how you'd create the component

  • How would you handle dynamic screen sizes?
  • What types of props are required?
  • How will you handle localization?
  • How/where will this component be initialized and how will it be triggered to show?

@thienlnam
Copy link
Contributor

thienlnam commented May 20, 2021

@kakajann
Could you post your 'proposal' here just as a way to go back over the steps you missed? Since you've already completed your PR this would just be the steps and list of things you completed in your PR. After that, I will assign you to the issue and get you hired on upwork

@kakajann
Copy link
Contributor

Proposal

  • I will create GrowlNotification component with three types (success, warning, error)
  • I will add necessary translations to en.js
  • and finally show it when a user make a change in their profile.

GrowlNotification component features:

  • responsive
  • reusable
  • have 3 types – success, warning, error
  • swipe to dismiss (in iOs and Android)
  • click to dismiss (web, desktop)

by the way, everything is ready in this PR and ready to merge.

@thienlnam
Copy link
Contributor

thienlnam commented May 20, 2021

Thanks, next time it would be good to see some information about how you would go about accomplishing those component features but your proposal has been accepted and I've hired you on upwork. Please make sure to complete the steps in order next time. #2944 (review)

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 a pull request may close this issue.

7 participants