Use formik for login and sign up forms #235

phyrog · 2017-12-06T12:46:09Z

Closes #161.

I had to remove some tests for the SignInForm and SignUpForm as I couldn't find any way on how to wait for validation/submission to finish to check the changes in output. This happens all asynchronously and I don't have access to the Promises, so I'm out of ideas here.

houndci-bot · 2017-12-06T12:46:24Z

src/components/SignUpForm/index.js

+          captcha.execute();
+        }
+        if (!enableCaptcha) {
+          console.error("Ok");


Unexpected console statement no-console

houndci-bot · 2017-12-06T12:46:25Z

formik.js

+import { size } from "lodash";
+
+function submitForm(values) {
+  console.log(values);


Unexpected console statement no-console

houndci-bot · 2017-12-06T12:46:25Z

formik.js

+import "./App.css";
+import { Formik } from "formik";
+import ReCAPTCHA from "react-google-recaptcha";
+import { size } from "lodash";


'size' is defined but never used no-unused-vars

houndci-bot · 2017-12-06T12:46:25Z

formik.js

@@ -0,0 +1,51 @@
+import React, { Component } from "react";
+import logo from "./logo.svg";


'logo' is defined but never used no-unused-vars

houndci-bot · 2017-12-06T12:46:25Z

formik.js

@@ -0,0 +1,51 @@
+import React, { Component } from "react";


'Component' is defined but never used no-unused-vars

codecov · 2017-12-06T12:46:27Z

Codecov Report

Merging #235 into master will increase coverage by 1.42%.
The diff coverage is 95.38%.

@@            Coverage Diff             @@
##           master     #235      +/-   ##
==========================================
+ Coverage   96.15%   97.58%   +1.42%     
==========================================
  Files          11       14       +3     
  Lines         156      124      -32     
  Branches       22       11      -11     
==========================================
- Hits          150      121      -29     
+ Misses          5        3       -2     
+ Partials        1        0       -1

Impacted Files	Coverage Δ
src/components/LoginModal.js	`100% <100%> (ø)`	⬆️
src/components/SignUpForm/Field.js	`100% <100%> (ø)`
src/components/GlobalMenu.js	`100% <100%> (ø)`	⬆️
src/components/SignUpForm/PasswordStrengthBar.js	`100% <100%> (ø)`
src/components/SignInForm.js	`100% <100%> (ø)`	⬆️
src/components/SignUpForm/index.js	`100% <100%> (ø)`
src/components/SignUpForm/withRefs.js	`83.33% <83.33%> (ø)`
src/components/SignUpForm/validation.js	`95.45% <95.45%> (ø)`
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a96fad...717f014. Read the comment docs.

phyrog · 2017-12-11T10:23:40Z

I'm currently reworking this to be better testable. Not ready for review yet.

houndci-bot · 2017-12-13T13:36:26Z

src/components/__stories__/SignUpForm.stories.js

+
+import { ThemeProvider } from "styled-components";
+import theme from "../../styles";
+import { Icon, Input, Header as SemHeader, Menu } from "semantic-ui-react";


'Icon' is defined but never used no-unused-vars
'Input' is defined but never used no-unused-vars
'SemHeader' is defined but never used no-unused-vars
'Menu' is defined but never used no-unused-vars

houndci-bot · 2017-12-13T13:36:26Z

src/components/__stories__/SignUpForm.stories.js

+import { action } from "@storybook/addon-actions";
+
+import { ThemeProvider } from "styled-components";
+import theme from "../../styles";


'theme' is defined but never used no-unused-vars

houndci-bot · 2017-12-13T13:36:26Z

src/components/__stories__/SignUpForm.stories.js

+import { storiesOf } from "@storybook/react";
+import { action } from "@storybook/addon-actions";
+
+import { ThemeProvider } from "styled-components";


'ThemeProvider' is defined but never used no-unused-vars

houndci-bot · 2017-12-13T13:36:26Z

src/components/SignUpForm/validation.js

+  let matches;
+  let regex = /GraphQL error:\s*((\S+) .+)\s*$/gm;
+
+  while ((matches = regex.exec(err.message))) {


Unexpected assignment within a 'while' statement no-cond-assign

This is wanted in this case.

Then, let's use

// eslint-disable-next-line no-cond-assign while ((matches = regex.exec(err.message))) {

houndci-bot · 2017-12-13T13:36:26Z

src/components/SignUpForm/validation.js

+  forEach,
+  compact,
+  capitalize,
+  trim


'trim' is defined but never used no-unused-vars

houndci-bot · 2017-12-13T13:36:27Z

src/components/SignUpForm/__tests__/validation.test.js

+  }
+}));
+
+import {


Import in body of module; reorder to top import/first

houndci-bot · 2017-12-13T13:36:27Z

src/components/SignUpForm/__tests__/validation.test.js

@@ -0,0 +1,139 @@
+jest.mock("../../../apollo", () => ({
+  Client: {
+    query: ({ variables: { id: id } }) => {


Destructuring assignment id unnecessarily renamed no-useless-rename

houndci-bot · 2017-12-13T13:36:27Z

src/components/SignUpForm/__tests__/index.test.js

+      "#sign-up-email",
+      "#sign-up-password",
+      "#sign-up-password-confirm"
+    ].map(selector => {


Expected to return a value in arrow function array-callback-return

houndci-bot · 2017-12-13T13:36:27Z

src/components/SignUpForm/__tests__/index.test.js

@@ -0,0 +1,111 @@
+import React from "react";
+import { shallow, mount } from "enzyme";
+import _ from "lodash";


'_' is defined but never used no-unused-vars

houndci-bot · 2017-12-13T13:36:27Z

src/components/SignUpForm/__tests__/index.test.js

@@ -0,0 +1,111 @@
+import React from "react";
+import { shallow, mount } from "enzyme";


'shallow' is defined but never used no-unused-vars

eugenk

This and my earlier hound-related comment.

eugenk · 2017-12-14T08:12:09Z

src/components/SignUpForm/index.js

+  touched,
+  values
+}) => (
+  <Form onSubmit={!submitDisabled ? onSubmit : undefined}>


Isn't it more readable with one less negation? ;)

submitDisabled ? undefined : onSubmit

or even:

submitEnabled ? onSubmit : undefined

phyrog · 2017-12-17T09:02:46Z

I rebased on master and added commit 717f014 to address the review.

eugenk

https://media.giphy.com/media/3o6fJbwc9F2lB7sHh6/giphy.gif

phyrog added the in progress label Dec 6, 2017

phyrog requested a review from eugenk December 6, 2017 12:46

houndci-bot reviewed Dec 6, 2017

View reviewed changes

phyrog force-pushed the formik branch 2 times, most recently from edef986 to 3bb6698 Compare December 6, 2017 12:47

houndci-bot reviewed Dec 13, 2017

View reviewed changes

phyrog force-pushed the formik branch 2 times, most recently from 2aeeb5c to 25a965a Compare December 13, 2017 13:44

eugenk requested changes Dec 14, 2017

View reviewed changes

phyrog force-pushed the formik branch from 1a6e6d6 to 5cb14e7 Compare December 17, 2017 09:01

Tom Gehrke added 10 commits December 17, 2017 10:04

Replace SignInForm

6715f95

Replace SignUpForm and remove tests

717472b

Refactor formik form and add some tests

ff949f5

Add tests

150e06d

Move component

bc9650e

Remove old validations

0274b3b

Obey hound

08718c5

Update travis node versions

c3245c2

Ignore some non-testable code from coverage

712144c

Obey review

717f014

phyrog force-pushed the formik branch from 5cb14e7 to 717f014 Compare December 17, 2017 09:05

eugenk approved these changes Dec 17, 2017

View reviewed changes

phyrog merged commit 3a81a19 into master Dec 17, 2017

phyrog deleted the formik branch December 17, 2017 20:51

phyrog removed the in progress label Dec 17, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use formik for login and sign up forms #235

Use formik for login and sign up forms #235

phyrog commented Dec 6, 2017

houndci-bot Dec 6, 2017

houndci-bot Dec 6, 2017

houndci-bot Dec 6, 2017

houndci-bot Dec 6, 2017

houndci-bot Dec 6, 2017

codecov bot commented Dec 6, 2017 •

edited

Loading

phyrog commented Dec 11, 2017

houndci-bot Dec 13, 2017

houndci-bot Dec 13, 2017

houndci-bot Dec 13, 2017

houndci-bot Dec 13, 2017

phyrog Dec 13, 2017

eugenk Dec 13, 2017 •

edited

Loading

houndci-bot Dec 13, 2017

houndci-bot Dec 13, 2017

houndci-bot Dec 13, 2017

houndci-bot Dec 13, 2017

houndci-bot Dec 13, 2017

houndci-bot Dec 13, 2017

eugenk left a comment

eugenk Dec 14, 2017

phyrog commented Dec 17, 2017 •

edited

Loading

eugenk left a comment •

edited

Loading

		@@ -0,0 +1,51 @@
		import React, { Component } from "react";
		import logo from "./logo.svg";

		@@ -0,0 +1,111 @@
		import React from "react";
		import { shallow, mount } from "enzyme";

Use formik for login and sign up forms #235

Use formik for login and sign up forms #235

Conversation

phyrog commented Dec 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Dec 6, 2017 • edited Loading

Codecov Report

phyrog commented Dec 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eugenk Dec 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eugenk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phyrog commented Dec 17, 2017 • edited Loading

eugenk left a comment • edited Loading

Choose a reason for hiding this comment

codecov bot commented Dec 6, 2017 •

edited

Loading

eugenk Dec 13, 2017 •

edited

Loading

phyrog commented Dec 17, 2017 •

edited

Loading

eugenk left a comment •

edited

Loading