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

Add FormInputValidation to G Suite Add User Field #29728

Merged
merged 10 commits into from
Jan 9, 2019

Conversation

cbauerman
Copy link
Contributor

@cbauerman cbauerman commented Dec 22, 2018

Changes proposed in this Pull Request

  • Use FormInputValidation in the "Add G Suite User" field

BEFORE
screen_shot_2019-01-08_at_2_42_03_pm

AFTER
screen_shot_2019-01-08_at_2_32_46_pm

Testing instructions

  1. Navigate to http://calypso.localhost:3000/domains/manage/email/:siteSlug: on a site with G Suite
  2. Click "Add G Suite User"
  3. Immediately click "Continue"
  4. Confirm that all 3 fields are highlighted with red and say "This field is required." below
  5. Fill in the username with a single character, like a, and press "Continue"
  6. Confirm that that the username field is not highlighted
  7. Replace the username with a& and press "Continue"
  8. Confirm that username field now has the error message "Only number, letters, dashes, underscores, apostrophes and periods are allowed."
  9. Now replace the username with 123456789123456789123456789123456789123456789123456789123456789123456789 and confirm the error message is "Please provide a valid email address.".

Fixes #29696

@matticbot
Copy link
Contributor

@cbauerman cbauerman self-assigned this Dec 22, 2018
@cbauerman cbauerman changed the title First pass on adding FormInputValidation Add FormInputValidation to G Suite Add User Field Jan 7, 2019
@cbauerman cbauerman added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Jan 7, 2019
Copy link
Contributor

@stephanethomas stephanethomas left a comment

Choose a reason for hiding this comment

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

The following error is throw when I access the Add G Suite User page:

Uncaught ReferenceError: kebabCase is not defined
    at eval (app:///./client/my-sites/domains/domain-management/add-google-apps/add-email-addresses-card.jsx:479)
    at Array.map (<anonymous>)
    at getDerivedStateFromProps (app:///./client/my-sites/domains/domain-management/add-google-apps/add-email-addresses-card.jsx:466)
    at applyDerivedStateFromProps (app:///./node_modules/react-dom/cjs/react-dom.development.js:12278)
    at mountClassInstance (app:///./node_modules/react-dom/cjs/react-dom.development.js:12630)
    at updateClassComponent (app:///./node_modules/react-dom/cjs/react-dom.development.js:14256)
    at beginWork (app:///./node_modules/react-dom/cjs/react-dom.development.js:15082)
    at performUnitOfWork (app:///./node_modules/react-dom/cjs/react-dom.development.js:17820)
    at workLoop (app:///./node_modules/react-dom/cjs/react-dom.development.js:17860)
    at HTMLUnknownElement.callCallback (app:///./node_modules/react-dom/cjs/react-dom.development.js:149)

Unit tests are failing as well.

@stephanethomas stephanethomas added [Status] Needs Author Reply G Suite and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jan 8, 2019
@cbauerman cbauerman force-pushed the add/gsuite-add-user-validation branch from 4a45c74 to 744a904 Compare January 8, 2019 17:50
@cbauerman cbauerman added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Author Reply labels Jan 8, 2019
@cbauerman cbauerman force-pushed the add/gsuite-add-user-validation branch from 98a658e to c935211 Compare January 8, 2019 19:24
@belcherj
Copy link
Contributor

belcherj commented Jan 8, 2019

Spacing looks off before you add the errors, too much space between fields:
screen shot 2019-01-08 at 3 04 27 pm

@belcherj
Copy link
Contributor

belcherj commented Jan 8, 2019

Looks like field width is tied to the error text. The last screenshot is what it should look like.

screen shot 2019-01-08 at 3 06 21 pm
screen shot 2019-01-08 at 3 06 42 pm
screen shot 2019-01-08 at 3 06 49 pm
screen shot 2019-01-08 at 3 07 09 pm

Copy link
Contributor

@belcherj belcherj left a comment

Choose a reason for hiding this comment

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

The CSS needs to be fixed up so that the errors don't mess with the field widths.

@cbauerman cbauerman force-pushed the add/gsuite-add-user-validation branch from c935211 to 0f51f68 Compare January 8, 2019 21:09
@belcherj
Copy link
Contributor

belcherj commented Jan 9, 2019

screen shot 2019-01-09 at 11 18 51 am

@belcherj belcherj self-requested a review January 9, 2019 17:28
@cbauerman
Copy link
Contributor Author

fieldset cannot flex.

Copy link
Contributor

@belcherj belcherj left a comment

Choose a reason for hiding this comment

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

Tested and code looks good

@cbauerman cbauerman added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jan 9, 2019
@cbauerman cbauerman merged commit 6bfc790 into master Jan 9, 2019
@cbauerman cbauerman deleted the add/gsuite-add-user-validation branch January 9, 2019 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants