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

Progressive Registration #3893

Conversation

suppermancool
Copy link
Contributor

Progressive Registration

Progressive Registration
@suppermancool suppermancool changed the base branch from dev to feature/progressive-registration April 13, 2020 18:24
@vikasrohit vikasrohit requested a review from maxceem April 14, 2020 09:38
Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

Thanks for super fast PR @suppermancool.

There are some issues though.

image

  1. We should keep showing the close (ESC) button.

  2. Name button to submit form "Send my Request".

  3. Next to the "Send my Request" button, show button "Back" which would lead to the previous step.

  4. when some required fields are not filled, we should show them immediately when we show form with error messages so user knows which fields to fill in

    At the moment when we open form errors are not shown for required but not filled fields:

    image

  5. Please, show the title and text as per design:

    image

  6. We should show the loading indicator during requesting user details, but at the moment it doesn't, see demo video

  7. Working hours are not filled and marked as required, but button to submit form become enabled

    image

    It shouldn't get enabled until we fill both fields for working hours.

  8. User which doesn't have any of the Topcoder roles should see another set of fields:

    image

    • Note that Topcoder User is not on the list of Topcoder roles.
  9. When we fill all the required user details fields and click Send my Request:

    • the loading indicator should be shown
    • after the form is submitted, the project should be directly created (loading indicator is still showing until the project is created)
    • the message that the project is created is shown like usual
    • see demo video how it's now https://monosnap.com/file/Qq6lXIka8fFOzKVuL8A4G14ggufllB

Progressive Registration Feedback for loading/role/field problem
@suppermancool
Copy link
Contributor Author

@maxceem done in c89607d

@maxceem maxceem self-requested a review April 16, 2020 15:43
@maxceem maxceem marked this pull request as draft April 16, 2020 15:44
@maxceem maxceem marked this pull request as ready for review April 16, 2020 15:44
Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

Thanks, @suppermancool. It's much closer to the aim now.

Most issues are fixed, but now I see other some issues so I didn't check code yet, only functionality for now.

  1. ✅done

  2. ✅done

  3. Back button now opens the first step of the form, but it has to open the last step of the form. Back button, in general, should move us one step back. See demo video https://monosnap.com/file/hBYZF4htpEBxp0VP93nBPTy0LnuN2k

  4. ✅done

  5. ✅done

  6. ❌ Done, but please, don't show the back and close buttons so the user cannot break the process. The same way as we do when we submit a project.

    Screenshot 2020-04-16 at 20 51 50
  7. ❌ I can still reproduce it. I use pshah_manager user, but I nullify traits on load for testing, see screenshot. All the fields are required, and when the form is shown messages are shown for all, but not for working hours:

    image

    I fill all the fields except of working hours and button to submit becomes enabled, but working hours are not filled, so the button should be still disabled:

    image

  8. ✅done

  9. ❌I user maxceem13 user (I manually sent request to clear phone first). I fill the form http://local.topcoder-dev.com:3000/new-project/app_new_intake and after I can see the form to complete user data. I fill it and click send my request, but the project is not created https://monosnap.com/file/I0X6TFGRwVrfXC2EkHnlbuNMTGVlfY

  10. After the fix when I open the user profile settings page, it makes an additional request to update user treaits https://monosnap.com/file/IMgMy42wHcSMmRSmmir6TMIWOGVYbC. But before it didn't https://monosnap.com/file/OO0O8OZrDWA3I90JpUisljvfmbNlaK.

  11. Back button doesn't work if we fill the form until the end and reload the page after. Now if we click Save my project we cannot come back, see demo https://monosnap.com/file/Qs5mPNvfDN94ynMdRtl2KUeTXHdy9U

  12. For Back and Send my Request buttons on this form:

    image

    please, use the same styles (size) as on the pages of project create form:

    image

  13. If I create a new project (with filling the missing fields on the way), and click button Go to project dashboard. And after I click a button in top right corner to create a new project I would be redirected to the page to choose a project type. But after that I would be redirected to the message that new project is created again. And new project indeed is automatically created. See demo video https://monosnap.com/file/Ir3EHwCjVvlyTN02As9k53OFAR0jHQ.

  14. In case of "Customer/Freelancer user" at the bottom of the screen, there should be a message that reads: “Were you looking to join Topcoder’s freelancer community and participate in work?” with a button that reads “Yes.” If the user clicks this button, the project should not be created and the user should be routed to the community app.

    image

  15. When I open user profile settings page, all the required fields show error message. But we shouldn't do it here, only during project creating:

    image

Progressive Registration Feedback for loading 2nd
@suppermancool
Copy link
Contributor Author

suppermancool commented Apr 17, 2020

@maxceem all done in the latest commit

For 7 done in appirio-tech/react-components#358.

@maxceem maxceem self-requested a review April 17, 2020 09:00
Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

Great @suppermancool, all previous issues 1 - 15 expect 3 are fixed!

  1. Back button now goes to the last step of the form as required. But if I try to change anything I immediately get redirected to the user details form, see demo video https://monosnap.com/file/pvamRpAaFzsD7BzpOnSGTNDpGhMuLr

I've noticed just a couple of minor UI issues.

  1. The form for entering user details during project creation is not centered like the rest of content, see https://monosnap.com/file/INpdSj1TORHKMDGWWc0OorVlESa92z and https://monosnap.com/file/xQtuGjeLsSDHmhih0QRyOWuvQRhqiR. I think we can set the width to 760px and center it as it's done on the User Profile page, see https://monosnap.com/file/AeYKzIWqsGtPI8BY7dF7108spIkbo2

  2. When I click Save my project first time. The loading indicator is shown good in the upper part of the page. But if on the user details form for Topcoder users click Back and then click Save my Project again, this time the loading indicator would be shown at the bottom part of the page and there would be unnecessary scrollbar, see demo video https://monosnap.com/file/sX1PoEnFrqANYmoZC2iCjlWLaZSBeS

  3. Probably connected with the previous one. If I click Save my project button when all the user data is already complete, first I see the loading indicator at the center and after it jumps to the top, see demo video https://monosnap.com/file/qbyN2X3Oybzl9tSFDTJiRsR7lBL0U9.

    • Can we use only use one style of loading indicator so it doesn't jump? It could be shown at the center or at the top, any place is good as long as it's the same all the time.

PS
The code looks good to me. The only thing the src/routes/settings/routes/profile/components/ProfileSettingsForm.jsx component becomes quite tricky with complex props structure and Back button which looks like doesn't belong there. We could create a new component for rendering forms during project creating, though in such case we would have to duplicate the logic which have in the ProfileSettingsForm component regarding county and phone number, which is also not good.

Do you have any idea of how to structure the code regarding this form in a better way? No immediate actions are needed, just trying to come with an idea of possible refactoring.

fix problem with loading poisition + hide/show profiile
@suppermancool
Copy link
Contributor Author

suppermancool commented Apr 17, 2020

@maxceem done in the latest commit

For src/routes/settings/routes/profile/components/ProfileSettingsForm.jsx:

I think we should create new child component for profile form. This component will have all the field, and have props to check which fields should be shown or required.
Then import this new component in both ProfileSettingsForm.jsx and projects/create/components/UpdateUserInfo.js

@maxceem maxceem self-requested a review April 20, 2020 07:07
Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

Works great now, @suppermancool.

Thanks, for your suggestion regarding refactoring, we would consider making it in the future.

@maxceem maxceem merged commit 412ad04 into appirio-tech:feature/progressive-registration Apr 20, 2020
@maxceem
Copy link
Collaborator

maxceem commented Apr 20, 2020

My code snippet which I used for testing (in case I would need to test it in the future):

    .then(data => {
      console.warn({ data })

      const connectTrait = _.find(data, { traitId: 'connect_info' })
      const basicInfo = _.find(data, { traitId: 'basic_info' })

      console.warn({ connectTrait })

      if (connectTrait) {
        connectTrait.traits.data[0].workingHourStart = null
        connectTrait.traits.data[0].workingHourEnd = null
        connectTrait.traits.data[0].businessPhone = null
        connectTrait.traits.data[0].title = null
        connectTrait.traits.data[0].country = null
        connectTrait.traits.data[0].timeZone = null
      }

      if (basicInfo) {
        // basicInfo.traits.data[0].firstName = null
        // basicInfo.traits.data[0].lastName = null
      }

      return data
    })

maxceem pushed a commit that referenced this pull request Apr 21, 2020
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.

2 participants