Phone verification #946

2018-01-15T14:36:06+01:00

liamcardenas commented

2018-01-15 14:36:06 +01:00

(Migrated from github.com)

not quite ready for review yet, still ironing out some things

kauffj (Migrated from github.com) reviewed 2018-01-15 14:36:06 +01:00

kauffj (Migrated from github.com) requested changes 2018-01-18 16:17:09 +01:00

src/renderer/component/userFieldNew/view.jsx

kauffj (Migrated from github.com) commented

2018-01-18 16:14:36 +01:00

Is there a particular reason we use a single component to do two (seemingly) distinct duties, as opposed to UserEmailNew and UserPhoneNew or equivalent?

Is there a particular reason we use a single component to do two (seemingly) distinct duties, as opposed to `UserEmailNew` and `UserPhoneNew` or equivalent?

kauffj (Migrated from github.com) commented

2018-01-18 16:16:07 +01:00

Does this have to be collected separately as opposed to letting them type a full number? If it is separate field, could it at least be an inline select next to the full number entry?

kauffj (Migrated from github.com) reviewed 2018-01-18 16:18:54 +01:00

src/renderer/modal/modalPhoneCollection/index.js

					
				@ -0,0 +1,22 @@

				import React from 'react';

				import * as settings from 'constants/settings';

kauffj (Migrated from github.com) commented

2018-01-18 16:18:53 +01:00

Is it correct to always bump the user to rewards from here?

kauffj commented

2018-01-18 16:22:46 +01:00

(Migrated from github.com)

@liamcardenas Also, I seem to no longer be prompted for my email immediately after the welcome popup.

liamcardenas (Migrated from github.com) reviewed 2018-01-18 17:30:44 +01:00

src/renderer/component/userFieldNew/view.jsx

liamcardenas (Migrated from github.com) commented

2018-01-18 17:30:44 +01:00

this was in the spirit of DRY a la our coding maxims.

There is less of a case for userFieldNew to be used for two field types, but userFieldVerify is basically the same for both types. I made userFieldNew to be consistent with that.

this was in the spirit of DRY a la our coding maxims. There is less of a case for userFieldNew to be used for two field types, but userFieldVerify is basically the same for both types. I made userFieldNew to be consistent with that.

liamcardenas (Migrated from github.com) reviewed 2018-01-18 17:35:06 +01:00

src/renderer/component/userFieldNew/view.jsx

liamcardenas (Migrated from github.com) commented

2018-01-18 17:35:05 +01:00

I suppose it could be a select. Twilio wants a separate country code. I tried to make this as fool proof and intuitive as possible. I think the way it is makes a lot of sense (it was based on a discussion in tech-app), but I am completely open to changing it.

liamcardenas (Migrated from github.com) reviewed 2018-01-18 17:38:36 +01:00

src/renderer/modal/modalPhoneCollection/index.js

					
				@ -0,0 +1,22 @@

				import React from 'react';

				import * as settings from 'constants/settings';

liamcardenas (Migrated from github.com) commented

2018-01-18 17:38:35 +01:00

I believe so, since the only place you can access this is from the verify identity page.

liamcardenas commented

2018-01-18 17:39:20 +01:00

(Migrated from github.com)

@kauffj oh, for some reason, I thought we were taking that out. Should I add it back in?

kauffj (Migrated from github.com) reviewed 2018-01-18 19:06:54 +01:00

src/renderer/modal/modalPhoneCollection/index.js

					
				@ -0,0 +1,22 @@

				import React from 'react';

				import * as settings from 'constants/settings';

kauffj (Migrated from github.com) commented

2018-01-18 19:06:54 +01:00

👍

:+1:

kauffj (Migrated from github.com) reviewed 2018-01-18 19:09:00 +01:00

src/renderer/component/userFieldNew/view.jsx

kauffj (Migrated from github.com) commented

2018-01-18 19:09:00 +01:00

I don't see very much savings here aside from maybe some boilerplate in index.js, and it adds complexity to declaration and handling changes. I'd split these.