Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

Add confirmation to warn user of the cost of second passphrase #435

Merged
merged 8 commits into from Dec 16, 2017
Merged

Add confirmation to warn user of the cost of second passphrase #435

merged 8 commits into from Dec 16, 2017

Conversation

ckhatri
Copy link
Contributor

@ckhatri ckhatri commented Dec 4, 2017

Some people on the subreddit were mentioning there wasn't any popup or information about how creating a second passphrase costs ark. This toast warning message should popup and warn them how much it'll cost!

@j-a-m-l j-a-m-l assigned j-a-m-l and unassigned j-a-m-l Dec 4, 2017
@j-a-m-l j-a-m-l self-requested a review December 4, 2017 17:48
Copy link
Contributor

@j-a-m-l j-a-m-l left a comment

Choose a reason for hiding this comment

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

I like the idea, but some people may ignore the notice message, so probably the best way to avoid that is:

  • Change the text, to reflect that it's necessary to back-up the passphrase AND pay n ARK.
  • Show a confirmation after clicking NEXT that informs that it's necessary to back-up the passphrase and pay n ARK.

@@ -1,7 +1,7 @@
module.exports = {
notice: {
logFile: false,
level: 1,
level: 2,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor Author

@ckhatri ckhatri Dec 4, 2017

Choose a reason for hiding this comment

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

The main reason for that change is because every time we send a toast message some message types are shown to the user such as error, success, but others are supposed to be logged only such as log and debug. I believe warning should be shown to the user so I made it level 2 instead of 1.

Currently it does

if (type > self.loggingType) {
self.logToFile(message, typeName)
return
}

When ya say a confirmation, do you mean a popup kinda so they click 'I agree' or something like that? Instead of a toast message?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, a confirmation would be a new intermediate screen after clicking "NEXT" on the same window, or a pop-up with "I agree" or something similar.

Copy link
Contributor Author

@ckhatri ckhatri Dec 13, 2017

Choose a reason for hiding this comment

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

Added an alert that shows before the secondPassphrase generation starts. They must hit OK before continuing.

The toast message is also there still.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good work.

I'd remove the toast message because toast notices should be a way to reflect the end of an action on the background, an error, etc. In this case, is unnecessary and it could even be distracting.

Apart from that, the only change I'd do is using different texts:
"Are you sure?" => "Second passphrase fee"
"WARNING! Second passphrase creation costs ' + secondPhraseArkVal + ' '+ currency +'.'"
"CREATE" => "CONTINUE"

Copy link
Contributor

Choose a reason for hiding this comment

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

BTW @ckhatri What do you think about #431 ?

Copy link
Contributor Author

@ckhatri ckhatri Dec 14, 2017

Choose a reason for hiding this comment

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

Should I keep the changes to the levels though? Since we do want toast warnings to be shown not logged? Or should I revert that back?

Agreed with the changes, I'll get those done soon.

EDIT: I agree, if there's a consensus to change it I'd be glad to get that fix rolled out!

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd remove entirely the toast message and I'd revert the notice configuration to level 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got rid of the toast message and reverted the notice!

@@ -1,7 +1,7 @@
module.exports = {
notice: {
logFile: false,
level: 1,
level: 2,
Copy link
Contributor

Choose a reason for hiding this comment

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

Good work.

I'd remove the toast message because toast notices should be a way to reflect the end of an action on the background, an error, etc. In this case, is unnecessary and it could even be distracting.

Apart from that, the only change I'd do is using different texts:
"Are you sure?" => "Second passphrase fee"
"WARNING! Second passphrase creation costs ' + secondPhraseArkVal + ' '+ currency +'.'"
"CREATE" => "CONTINUE"

@j-a-m-l j-a-m-l changed the title added toast message to warn user of second passphrase Add confirmation to warn user of the cost of second passphrase Dec 16, 2017
@j-a-m-l j-a-m-l merged commit 115c53f into ArkEcosystem:master Dec 16, 2017
@j-a-m-l
Copy link
Contributor

j-a-m-l commented Dec 16, 2017

+5 Good work!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants