Add checkbox warning #2909

vctt94 · 2020-11-11T19:54:07Z

This PR adds a checkbox for allowing sending transaction from other accounts than mixed account. closes #2877

amass01

LGTM added one suggestion 👍

amass01 · 2020-11-11T20:26:12Z

app/components/views/PrivacyPage/Privacy/Privacy.jsx

  } = usePrivacy();

  const [logs, setLogs] = useState("");
+  const [showingSendUnmixModal, showModal] = useState(false);
+  const onToggleSendFromUnmixed = () => {


useCallback here might be useful

it will be a callback with multiple function methods on its array dependency, which will be re-rendered each update. That's why I didn't add

hrrmmm i'm not sure if i understand, useCallback will prevent extra re-renders in children components (the one receives onToggleSendFromUnmixed) it will guarantee to keep the same pointer to functions unless one of the deps array updates, in that case it will re-evaluate the useCallback value which will result in new pointer and children components wil re-render, without useCallback each re-render in parent will cause re-evaluation of the function which means new pointer which means children will re-render as well.

Functions are comparing by reference, so if I add a function at the array dependency, it will not have much effect. Here: https://hackernoon.com/grasp-by-value-and-by-reference-in-javascript-7ed75efa1293

So I see no reason for adding it. We will not increase performance and the method will be more verbose.

oh i see, didn't notice these are functions.

vctt94 · 2020-11-11T20:30:49Z

app/components/views/PrivacyPage/Privacy/Privacy.jsx

+      showModal(false);
+    }
+  };
+  const onChangeCheckbox = () => {


see above comment

vctt94 · 2020-11-11T20:30:49Z

app/components/views/PrivacyPage/Privacy/Privacy.jsx

+      showModal(false);
+    }
+  };
+  const onChangeCheckbox = () => {


victorgcramos

Good job! Working as expected. Just a few minor visual issues that I noticed:

modal background should be a little darker
components and files with SeedCopyConfirmModal naming

jholdstock

I just loaded up a wallet which already has privacy enabled and the checkbox was not checked. Perhaps it is using the wrong default value when no value is found in the config file?
If I press "Cancel" in the modal, the checkbox value changes anyway

jholdstock · 2020-11-12T09:13:53Z

app/components/modals/SendFromUnmixedAccountModal/SendFromUnmixedAccountContent.jsx

+}) => (
+  <DefaultModal className={style.confirmSendFromUnmixed} {...{ show }}>
+    <div className={style.titleConfirmSendFromUnmixed}>
+      <T id="SendFromUnmixed.titleWarning" m="Seed Clipboard Copy Warning" />


Suggested change

<T id="SendFromUnmixed.titleWarning" m="Seed Clipboard Copy Warning" />

<T id="SendFromUnmixed.titleWarning" m="Sending from Unmixed Accounts" />

jholdstock · 2020-11-12T09:14:45Z

app/components/modals/SendFromUnmixedAccountModal/SendFromUnmixedAccountContent.jsx

+          typedConfirmationPhrase.toLowerCase() !==
+          copyConfirmationPhrase.toLowerCase()
+        }>
+        <T id="SendFromUnmixed.btnConfirm" m="Allow send from Unmixed" />


Suggested change

<T id="SendFromUnmixed.btnConfirm" m="Allow send from Unmixed" />

<T id="SendFromUnmixed.btnConfirm" m="Enable sending from unmixed accounts" />

jholdstock · 2020-11-12T09:17:52Z

app/components/views/PrivacyPage/Privacy/PrivacyContent.jsx

+      label={
+        <T
+          id="Privacy.checkbox"
+          m="Send From Unmixed accounts"


Suggested change

m="Send From Unmixed accounts"

m="Send from unmixed accounts"

jholdstock · 2020-11-12T09:20:09Z

app/components/views/PrivacyPage/Privacy/PrivacyContent.jsx

+    <Checkbox
+      label={
+        <T
+          id="Privacy.checkbox"


I think this is the only string ID I have seen starting with a capital letter. We can give it a more descriptive name too, in case we add another checkbox later.

Suggested change

id="Privacy.checkbox"

id="privacy.sendFromUnmixedCheckbox"

jholdstock · 2020-11-12T09:22:45Z

app/i18n/docs/en/InfoModals/SendFromUnmixed.md

@@ -0,0 +1,3 @@
+## Allow Send from other unmixed accounts


I think we can remove this heading, because the modal already has one. This one isn't rendered properly anyway, it just appears as a bold line at the start of the paragraph.

jholdstock · 2020-11-12T09:34:30Z

app/i18n/docs/en/InfoModals/SendFromUnmixed.md

@@ -0,0 +1,3 @@
+## Allow Send from other unmixed accounts
+
+Currently, you have privacy settings enabled.  As part of this, we have turned off the ability to create external spends from non-mixed accounts.  Any funds sent from the non-mixed account will not be private and will be able to be tracked.  If you would like to turn off this setting please type in 'I understand the risks' below.  You may re-engage this setting at any time.


Suggested change

Currently, you have privacy settings enabled. As part of this, we have turned off the ability to create external spends from non-mixed accounts. Any funds sent from the non-mixed account will not be private and will be able to be tracked. If you would like to turn off this setting please type in 'I understand the risks' below. You may re-engage this setting at any time.

Sending from unmixed accounts has been disabled by the privacy settings of this wallet. This is for your protection.

Sending from unmixed accounts is not private and could possibly be traced back to you. If you would like to turn off this setting please type 'I understand the risks' below. You may re-engage this setting at any time.

amass01 approved these changes Nov 11, 2020

View reviewed changes

vctt94 commented Nov 11, 2020

View reviewed changes

app/components/views/PrivacyPage/Privacy/Privacy.jsx

showModal(false);

}

};

const onChangeCheckbox = () => {

Copy link

Member Author

vctt94 Nov 11, 2020

Choose a reason for hiding this comment

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

same

victorgcramos suggested changes Nov 11, 2020

View reviewed changes

vctt94 force-pushed the add-checkbox-warning branch from 5862ccc to b91d5c6 Compare November 11, 2020 22:11

vctt94 added 8 commits November 11, 2020 19:47

Add checkbox and logic

1c2add1

Add modal

0afa28d

Fix wrong wallet config param

b764254

lint

cfe0389

fix dupe

45d2dc2

Fix background size and naming

e3f4f95

small nits

c078c99

properly filter tickets

6a2d562

vctt94 force-pushed the add-checkbox-warning branch from 6e249c9 to 6a2d562 Compare November 11, 2020 22:50

Reduce modal size

f81fdae

jholdstock suggested changes Nov 12, 2020

View reviewed changes

vctt94 added 2 commits November 12, 2020 09:41

Add send_from_unmixed initial value

88f3db3

review fixes

cd42e85

alexlyp approved these changes Nov 12, 2020

View reviewed changes

jholdstock approved these changes Nov 12, 2020

View reviewed changes

alexlyp merged commit 4765996 into decred:master Nov 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add checkbox warning #2909

Add checkbox warning #2909

vctt94 commented Nov 11, 2020 •

edited

Loading

amass01 left a comment

amass01 Nov 11, 2020

vctt94 Nov 11, 2020

amass01 Nov 11, 2020 •

edited

Loading

vctt94 Nov 11, 2020

amass01 Nov 11, 2020

vctt94 Nov 11, 2020

amass01 Nov 11, 2020

vctt94 Nov 11, 2020

victorgcramos left a comment

jholdstock left a comment

jholdstock Nov 12, 2020

jholdstock Nov 12, 2020

jholdstock Nov 12, 2020

jholdstock Nov 12, 2020

jholdstock Nov 12, 2020

jholdstock Nov 12, 2020

	<T id="SendFromUnmixed.titleWarning" m="Seed Clipboard Copy Warning" />
	<T id="SendFromUnmixed.titleWarning" m="Sending from Unmixed Accounts" />

	<T id="SendFromUnmixed.btnConfirm" m="Allow send from Unmixed" />
	<T id="SendFromUnmixed.btnConfirm" m="Enable sending from unmixed accounts" />

	m="Send From Unmixed accounts"
	m="Send from unmixed accounts"

		@@ -0,0 +1,3 @@
		## Allow Send from other unmixed accounts

		Currently, you have privacy settings enabled. As part of this, we have turned off the ability to create external spends from non-mixed accounts. Any funds sent from the non-mixed account will not be private and will be able to be tracked. If you would like to turn off this setting please type in 'I understand the risks' below. You may re-engage this setting at any time.

-Currently, you have privacy settings enabled.  As part of this, we have turned off the ability to create external spends from non-mixed accounts.  Any funds sent from the non-mixed account will not be private and will be able to be tracked.  If you would like to turn off this setting please type in 'I understand the risks' below.  You may re-engage this setting at any time.
+Sending from unmixed accounts has been disabled by the privacy settings of this wallet. This is for your protection.
+Sending from unmixed accounts is not private and could possibly be traced back to you.  If you would like to turn off this setting please type 'I understand the risks' below.  You may re-engage this setting at any time.

Add checkbox warning #2909

Add checkbox warning #2909

Conversation

vctt94 commented Nov 11, 2020 • edited Loading

amass01 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amass01 Nov 11, 2020 • 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

victorgcramos left a comment

Choose a reason for hiding this comment

jholdstock left a 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

Choose a reason for hiding this comment

vctt94 commented Nov 11, 2020 •

edited

Loading

amass01 Nov 11, 2020 •

edited

Loading