LBRYCommunity/lbry-desktop

Fork 0

Adding ability to resend verification emails #1560

Merged

amitnndn merged 7 commits from master into master

2018-06-08 21:50:53 +02:00

amitnndn commented

2018-06-07 05:07:14 +02:00

(Migrated from github.com)

#1492 - Ability to Resend verification emails.

Added the ability to resend verification emails when on the screen where the user enters the verification code that they receive via email.

This is still a WIP and I just wanted to get an early review of the code and placement of the 'Resend Verification Email' button.

I have some questions/thoughts off the top of my head:

Should the Resend Verification Email button disabled after the user clicks it for a specific amount of time? Just to avoid multiple emails being dispatched or overloading the API with multiple requests.
Do we need any signifying text next to the button maybe explaining what it does? (I know this is a bad design, but I just wanted to get all my thoughts out)
Maybe this is another issue or an improvement: There is no way that I can change my email through the verify email page. It would be nice to be able to change the email from the verification screen as well.

Preview

P.S: This is one of the first React and Redux apps that I am working on, still trying to grasp the concepts of reducers, actions, selectors etc., forgive me if there are any silly mistakes! :-)

#1492 - Ability to Resend verification emails. Added the ability to resend verification emails when on the screen where the user enters the verification code that they receive via email. This is still a WIP and I just wanted to get an early review of the code and placement of the 'Resend Verification Email' button. I have some questions/thoughts off the top of my head: * Should the Resend Verification Email button disabled after the user clicks it for a specific amount of time? Just to avoid multiple emails being dispatched or overloading the API with multiple requests. * Do we need any signifying text next to the button maybe explaining what it does? (I know this is a bad design, but I just wanted to get all my thoughts out) * Maybe this is another issue or an improvement: There is no way that I can change my email through the verify email page. It would be nice to be able to change the email from the verification screen as well. ### Preview ![screen shot 2018-06-06 at 9 04 46 pm](https://user-images.githubusercontent.com/4067311/41076176-ea74dc90-69d5-11e8-9964-a02fdba6dc3c.png) P.S: This is one of the first React and Redux apps that I am working on, still trying to grasp the concepts of reducers, actions, selectors etc., forgive me if there are any silly mistakes! :-)

amitnndn commented

2018-06-07 05:25:15 +02:00

(Migrated from github.com)

Oops! Looks like a connectivity issue broke the build! Bad omen for my first PR! :D

neb-b (Migrated from github.com) requested changes 2018-06-08 06:29:43 +02:00

neb-b (Migrated from github.com) left a comment

@amitnndn This is great! I didn't even know about the resend_token api call. Just a couple comments, the resend button should probably use the button="link" prop so it isn't as far to to the right. Also only the word "Resend" should be capitalized. "Resend verification email"

I don't think we need any sort of help text, or to disable the button for a certain time. It is pretty straightforward and our emails are usually sent pretty fast.

Great work!

@amitnndn This is great! I didn't even know about the `resend_token` api call. Just a couple comments, the resend button should probably use the `button="link"` prop so it isn't as far to to the right. Also only the word `"Resend"` should be capitalized. `"Resend verification email"` I don't think we need any sort of help text, or to disable the button for a certain time. It is pretty straightforward and our emails are usually sent pretty fast. Great work!

amitnndn commented

2018-06-08 07:15:17 +02:00

(Migrated from github.com)

Thank you @seanyesmunt! I have changed updated the code as per your comments. Do you have any idea why the Travis CI builds might be failing? It shows a connectivity error, I am not sure if something I added is breaking it.

neb-b commented

2018-06-08 07:17:52 +02:00

(Migrated from github.com)

@amitnndn Looks like it's running now. Sometimes Travis just acts a little wonky 🙂

neb-b (Migrated from github.com) approved these changes 2018-06-08 08:42:14 +02:00

neb-b commented

2018-06-08 08:42:55 +02:00

(Migrated from github.com)

@amitnndn build passed 🙂

I'll give this a test tomorrow morning but looks good.

@amitnndn build passed 🙂 I'll give this a test tomorrow morning but looks good.

amitnndn commented

2018-06-08 15:13:40 +02:00

(Migrated from github.com)

@seanyesmunt Thank you! Let me know if you have any questions!

tzarebczan commented

2018-06-08 21:20:56 +02:00

(Migrated from github.com)

@amitnndn, thanks for your first contribution! Please review our contributing guide if you have not done so already. Also, click here if you are interested in an LBC tip for your work :)

We'll also be adding you to our developer mailing list so we can keep in touch (you can unsubscribe anytime!).

To answer your question about changing the email, the short-term fix for that would be: https://github.com/lbryio/lbry-app/issues/997 (longer term - https://github.com/lbryio/lbry-app/issues/926)

@amitnndn, thanks for your first contribution! Please review our [contributing guide](https://lbry.io/faq/contributing) if you have not done so already. Also, [click here](https://lbry.io/faq/tips) if you are interested in an LBC tip for your work :) We'll also be adding you to our developer mailing list so we can keep in touch (you can unsubscribe anytime!). To answer your question about changing the email, the short-term fix for that would be: https://github.com/lbryio/lbry-app/issues/997 (longer term - https://github.com/lbryio/lbry-app/issues/926)

neb-b commented

2018-06-08 21:50:47 +02:00

(Migrated from github.com)

Working great! Thanks for the contribution! 🙂