dcr: Reimplemint privacy screens. #248

JoeGruffins · 2023-11-13T13:05:20Z

dcr: Reimplemint privacy screens.

dcr: Use default account on creation.

In order to make mixing opt in, start with it off and do not create the
accounts. Allow sending from the default account on creation.

closes #171

ui/page/privacy/manual_mixer_setup_page.go

JoeGruffins · 2023-11-13T13:14:25Z

ui/page/privacy/manual_mixer_setup_page.go

-											}),
-											layout.Rigid(func(gtx C) D {
-												txt := `<span style="text-color: grayText2">
-											<b>Make sure to select the same accounts from the previous privacy setup. </b><br>Failing to do so could compromise wallet privacy.<br> You may not select the same account for mixed and unmixed.


While this would allow to bold text in the middle of a sentence, the font ends up being different from the rest of the page so not using. Could also make everything html...

ui/page/privacy/setup_mixer_accounts_page.go

ui/values/localizable/en.go

JoeGruffins · 2023-11-13T13:16:45Z

ui/page/components/wallet_setup_page.go

-				dcrUniqueImpl := wal.(*dcr.Asset)
-				err = dcrUniqueImpl.CreateMixerAccounts(values.String(values.StrMixed), values.String(values.StrUnmixed), pg.passwordEditor.Editor.Text())
-				if err != nil {
-					errModal := modal.NewErrorModal(pg.Load, err.Error(), modal.DefaultClickFunc())
-					pg.ParentWindow().ShowModal(errModal)
-					return
-				}
-				wal.SetBoolConfigValueForKey(sharedW.AccountMixerConfigSet, true)
-


So not creating the accounts until the user turns mixing on.

ui/page/privacy/manual_mixer_setup_page.go

JoeGruffins · 2023-11-13T14:04:29Z

Current screenshots on desktop, yet to try mobile:

JoeGruffins

Oh its broken on mobile, should this pr worry with that?

JoeGruffins · 2023-11-14T07:12:18Z

Fixing most of the TODOs https://github.com/crypto-power/cryptopower/compare/2bb9f5ff465caac886c9dad280b6777f56b83fb3..a49d0ec318d0122b549cc9d5915815830317ee89

JoeGruffins · 2023-11-14T07:18:05Z

Fixed the spacing around the manual screen back arrow https://github.com/crypto-power/cryptopower/compare/a49d0ec318d0122b549cc9d5915815830317ee89..befab2bf2ef08de596310212857243a6b593b33c

dreacot · 2023-11-14T07:32:59Z

Oh its broken on mobile, should this pr worry with that?

usually mobile work should be focused for a different PR

but feel free to include it here if you want, i see the figma screens are ready

JoeGruffins · 2023-11-15T02:06:33Z

but feel free to include it here if you want, i see the figma screens are ready

Another pr would be good if that's alright, this one focuses on desktop. This solves the main intent of the issue I was trying to make, that privacy should be opt in.

Actually I forgot about the default receive account after you turn on can't send from default. Let me check that out will probably add more to this pr.

JoeGruffins · 2023-11-15T04:59:16Z

The default sending account is already ok. Made it impossible to toggle off the spend unmixed funds if mixing is not set up. https://github.com/crypto-power/cryptopower/compare/befab2bf2ef08de596310212857243a6b593b33c..28be9d560bcf5ab875620b4aa37524419980e297

Sirmorrison

First pass. Please add screenshots of the UI developed to the PR desc.

Sirmorrison · 2023-11-22T10:04:59Z

ui/page/privacy/manual_mixer_setup_page.go

+			return layout.Flex{Axis: layout.Vertical, Alignment: layout.Start}.Layout(gtx,
+				layout.Rigid(func(gtx C) D {


This lines are not needed and seem repeated. I would recommend using the linear Layout widget for this though.

Oh yeah, was able to remove the Flex layout here and it looks the same.

Linear in place of the Center below or the Card above?

ui/page/privacy/manual_mixer_setup_page.go

Sirmorrison · 2023-11-22T10:13:17Z

ui/page/privacy/manual_mixer_setup_page.go

+					layout.Flexed(1, func(gtx C) D {
+						return layout.Flex{Axis: layout.Vertical, Alignment: layout.Middle, Spacing: layout.SpaceAround}.Layout(gtx,
+							layout.Rigid(func(gtx C) D {
+								gtx.Constraints.Max.X = int(values.MarginPadding40)
+								return pg.Theme.Icons.ActionInfo.Layout(gtx, pg.Theme.Color.Gray1)
+							}),
+						)
+					}),
+					layout.Flexed(7, func(gtx C) D {


Flexed(1) and Flexed(7) seems off.

What would be good values here.

@JoeGruffins I think it just:

return layout.Flex{Alignment: layout.Start}.Layout(gtx, layout.Rigid(func(gtx C) D { gtx.Constraints.Max.X = int(values.MarginPadding40) return layout.Center.Layout(gtx, func(gtx layout.Context) layout.Dimensions { return pg.Theme.Icons.ActionInfo.Layout(gtx, pg.Theme.Color.Gray1) }) }), layout.Rigid(func(gtx C) D { return layout.Inset{ Left: values.MarginPadding10, }.Layout(gtx, func(gtx C) D { return layout.Flex{Axis: layout.Vertical}.Layout(gtx, layout.Rigid(func(gtx C) D { label := pg.Theme.H6(values.String(values.StrSetUpStakeShuffleWarningTitle)) return label.Layout(gtx) }), layout.Rigid(func(gtx C) D { label := pg.Theme.Body1(values.String(values.StrSetUpStakeShuffleWarningDesc)) return label.Layout(gtx) }), ) }) }), )

this is my suggestion code.

Sirmorrison · 2023-11-22T10:16:52Z

ui/page/privacy/setup_mixer_accounts_page.go

+			return layout.Flex{Axis: layout.Vertical, Alignment: layout.Middle}.Layout(gtx,
+				layout.Rigid(func(gtx C) D {
+					return layout.Center.Layout(gtx, func(gtx C) D {
+						return layout.Inset{Top: values.MarginPadding25}.Layout(gtx, func(gtx C) D {
+							return layout.Flex{Axis: layout.Vertical, Alignment: layout.Start}.Layout(gtx,


Using the linear layout widget will technically remove these lines of code as you can align the layout by passing the necessary params.

Was able to remove the Flex further above like you suggested in another place.

Linear in place of Center?

ui/values/localizable/en.go

ui/values/strings.go

ui/page/privacy/manual_mixer_setup_page.go

dreacot · 2023-11-22T22:52:34Z

the top margin is too much, i can't also see the button

dreacot · 2023-11-22T22:55:11Z

i get this while trying to setup manual mixer, i don't think the user should have funds in order to create a mixer

dreacot · 2023-11-22T22:57:24Z

trying auto setup, i selected auto move funds (i don't have any funds to move, so i assume that should be skipped)

clicking ok, i get stuck on this screen

JoeGruffins · 2023-11-24T07:33:37Z

Fixing all but three of the inline comments so far https://github.com/crypto-power/cryptopower/compare/b2dd44e2eac4e31ab45a48699ffa3845a8fc1ac7..4b6cb728c2272b91dfa10e73598ff8fbfde8406c

JoeGruffins · 2023-11-24T07:34:39Z

i get this while trying to setup manual mixer, i don't think the user should have funds in order to create a mixer

@dreacot Not funds, accounts. You need three accounts to set up mixing. Automatic will create them but if manual you already need them. I will look into the other error now.

JoeGruffins · 2023-11-24T08:30:36Z

@dreacot hopefully this fixes the no funds bug https://github.com/crypto-power/cryptopower/compare/4b6cb728c2272b91dfa10e73598ff8fbfde8406c..6f807e50846b3423fb9e7b49ce14cc2dae0254a1

dreacot · 2023-11-29T17:53:49Z

i'm still having the issue where i can't see the button

dreacot · 2023-11-29T17:55:06Z

@dreacot hopefully this fixes the no funds bug https://github.com/crypto-power/cryptopower/compare/4b6cb728c2272b91dfa10e73598ff8fbfde8406c..6f807e50846b3423fb9e7b49ce14cc2dae0254a1

it does

JoeGruffins · 2023-11-30T04:39:18Z

i'm still having the issue where i can't see the button

What button.

dreacot · 2023-11-30T04:40:17Z

the top margin is too much, i can't also see the button

This the button to start mixer setup

JoeGruffins · 2023-11-30T04:44:16Z

This the button to start mixer setup

Ohh. On desktop? I guess the card needs a set minimum thats higher than it is currently

dreacot · 2023-11-30T04:50:13Z

This the button to start mixer setup

Ohh. On desktop? I guess the card needs a set minimum thats higher than it is currently

I think the top padding is too much as well

JoeGruffins · 2023-11-30T06:18:26Z

@dreacot reduced a lot of the space https://github.com/crypto-power/cryptopower/compare/379d9f3c1fcfbe9ea63e7f49ebd9160689332b31..c63e28790fbd65d581fd10524348bc1fd62c80a5

Setting a max Y in the layout doesn't seem to be respected. These changes make the buttons viewable with as small a screen as I can make. I guess there is a minimum size I cannot makes the window smaller than?

dreacot · 2023-12-05T17:43:25Z

@dreacot reduced a lot of the space https://github.com/crypto-power/cryptopower/compare/379d9f3c1fcfbe9ea63e7f49ebd9160689332b31..c63e28790fbd65d581fd10524348bc1fd62c80a5

Setting a max Y in the layout doesn't seem to be respected. These changes make the buttons viewable with as small a screen as I can make. I guess there is a minimum size I cannot makes the window smaller than?

the app window? yeah its 650px minimum

JoeGruffins · 2023-12-06T05:41:20Z

I just rebased and there's still conflicts lol

JustinBeBoy

I don't think click title row is back button is good idea

JustinBeBoy · 2023-12-06T09:53:22Z

libwallet/assets/dcr/account_mixer.go

@@ -77,6 +77,7 @@ func (asset *Asset) CreateMixerAccounts(mixedAccount, unmixedAccount, privPass s
 	asset.SetInt32ConfigValueForKey(sharedW.AccountMixerMixedAccount, mixedAccountNumber)
 	asset.SetInt32ConfigValueForKey(sharedW.AccountMixerUnmixedAccount, unmixedAccountNumber)
 	asset.SetBoolConfigValueForKey(sharedW.AccountMixerConfigSet, true)
+	asset.SetBoolConfigValueForKey(sharedW.SpendUnmixedFundsKey, false)


set false for sharedW.SpendUnmixedFundsKey not need because when read it, default value return is false

I set the default to true in this commit I believe. Please see the CreateNewDCRWallet variants in libwallet/dcr.go.

JustinBeBoy · 2023-12-06T09:53:35Z

libwallet/assets/dcr/account_mixer.go

@@ -108,6 +109,7 @@ func (asset *Asset) SetAccountMixerConfig(mixedAccount, unmixedAccount int32, pr
 	asset.SetInt32ConfigValueForKey(sharedW.AccountMixerMixedAccount, mixedAccount)
 	asset.SetInt32ConfigValueForKey(sharedW.AccountMixerUnmixedAccount, unmixedAccount)
 	asset.SetBoolConfigValueForKey(sharedW.AccountMixerConfigSet, true)
+	asset.SetBoolConfigValueForKey(sharedW.SpendUnmixedFundsKey, false)


JoeGruffins · 2023-12-07T01:22:27Z

I don't think click title row is back button is good idea

Do you suggest just the arrow or arrow and "Manual Setup" label

I did the entire row because it's easier to click on mobile, if these screens can be reused.

dreacot · 2023-12-07T19:42:17Z

I don't think click title row is back button is good idea

Do you suggest just the arrow or arrow and "Manual Setup" label

I did the entire row because it's easier to click on mobile, if these screens can be reused.

i think the arrow and manual setup label should be fine

JoeGruffins · 2023-12-08T04:17:41Z

Just the arrow and label area are clickable. Does it look alright? https://github.com/crypto-power/cryptopower/compare/5aa3fff4a054bd4a938b2c35f6e797a64b51d641..c8d5703a4f149541dd003dcc6d8db35eaf2d50c8

JustinBeBoy

2 arrow seem like distorted and blurred

ui/page/privacy/manual_mixer_setup_page.go

JustinBeBoy · 2023-12-08T11:16:23Z

ui/page/privacy/setup_mixer_accounts_page.go

+		return layout.Inset{Top: values.MarginPadding25}.Layout(gtx, func(gtx C) D {
+			return layout.Flex{Axis: layout.Vertical, Alignment: layout.Start}.Layout(gtx,
+				layout.Rigid(func(gtx C) D {
+					label := pg.Theme.H6(values.String(values.StrSetUpStakeShuffleAutoOrManualA))
+					return layout.Inset{
+						Left: values.MarginPadding24,
+					}.Layout(gtx, label.Layout)
+				}),
+				layout.Rigid(func(gtx C) D {
+					label := pg.Theme.Body1(values.String(values.StrSetUpStakeShuffleAutoOrManualB))
+					return layout.Inset{
+						Left: values.MarginPadding24,
+					}.Layout(gtx, label.Layout)
+				}),
+				layout.Rigid(func(gtx C) D {
+					// TODO: Find a way to make Mixed and Unmixed bold while keeping to the theme.
+					label := pg.Theme.Body1(values.String(values.StrSetUpStakeShuffleAutoOrManualC))
+					return layout.Inset{
+						Top:  values.MarginPadding10,
+						Left: values.MarginPadding24,
+					}.Layout(gtx, label.Layout)
+				}),
+				layout.Rigid(func(gtx C) D {
+					label := pg.Theme.Body1(values.String(values.StrSetUpStakeShuffleAutoOrManualD))
+					return layout.Inset{
+						Top:  values.MarginPadding10,
+						Left: values.MarginPadding24,
+					}.Layout(gtx, label.Layout)
+				}),
+				layout.Rigid(func(gtx C) D {
+					return layout.Spacer{Height: values.MarginPadding80}.Layout(gtx)
+				}),
+				layout.Rigid(func(gtx C) D {
+					line := pg.Theme.Separator()
+					return layout.Inset{
+						Top:   values.MarginPadding10,
+						Left:  values.MarginPadding24,
+						Right: values.MarginPadding24,
+					}.Layout(gtx, line.Layout)
+				}),
+				layout.Rigid(func(gtx C) D {
+					return pg.autoSetupClickable.Layout(gtx, pg.autoSetupLayout)
+				}),
+				layout.Rigid(func(gtx C) D {
+					return pg.manualSetupClickable.Layout(gtx, pg.manualSetupLayout)
+				}),
+				layout.Rigid(func(gtx C) D {
+					return layout.Spacer{Height: values.MarginPadding10}.Layout(gtx)
+				}),
+			)
+		})


too many repeated Insets

you can try:

return layout.Inset{ Top: values.MarginPadding25, Left: values.MarginPadding24, Right: values.MarginPadding24, }.Layout(gtx, func(gtx C) D {...

Sure. Prevents the buttons from covering the whole width of the screen but I guess thats fine.

ui/page/privacy/setup_mixer_accounts_page.go

ui/page/privacy/setup_privacy_page.go

In order to make mixing opt in, start with it off and do not create the accounts. Allow sending from the default account on creation.

JoeGruffins · 2023-12-11T08:39:43Z

using a lot of code from @JustinBeBoy https://github.com/crypto-power/cryptopower/compare/3822f667033d78e0b477b5197bfd898f0a687fc7..da8ba79152ffc3cb7759f6cfa576401fd5a5404f

Do these arrows look ok?

ui/page/privacy/manual_mixer_setup_page.go

JoeGruffins · 2023-12-12T03:03:09Z

int() -> gtx.Dp() https://github.com/crypto-power/cryptopower/compare/da8ba79152ffc3cb7759f6cfa576401fd5a5404f..ec869d05893efb1ad871773b95d20f7be18f1cdf

Resolved

JoeGruffins force-pushed the addmixerscreens branch from 3f385c0 to 2bb9f5f Compare November 13, 2023 13:07

JoeGruffins commented Nov 13, 2023

View reviewed changes

JoeGruffins commented Nov 14, 2023

View reviewed changes

JoeGruffins force-pushed the addmixerscreens branch from 2bb9f5f to a49d0ec Compare November 14, 2023 07:11

JoeGruffins marked this pull request as ready for review November 14, 2023 07:12

JoeGruffins force-pushed the addmixerscreens branch from a49d0ec to befab2b Compare November 14, 2023 07:17

JoeGruffins force-pushed the addmixerscreens branch from befab2b to 28be9d5 Compare November 15, 2023 04:54

Sirmorrison suggested changes Nov 22, 2023

View reviewed changes

JustinBeBoy requested changes Nov 22, 2023

View reviewed changes

JoeGruffins force-pushed the addmixerscreens branch 2 times, most recently from b2dd44e to 4b6cb72 Compare November 24, 2023 07:31

JoeGruffins force-pushed the addmixerscreens branch from 4b6cb72 to 6f807e5 Compare November 24, 2023 08:29

JoeGruffins force-pushed the addmixerscreens branch 2 times, most recently from 379d9f3 to c63e287 Compare November 30, 2023 06:05

JoeGruffins force-pushed the addmixerscreens branch from c63e287 to c74b566 Compare December 6, 2023 04:23

JoeGruffins force-pushed the addmixerscreens branch from c74b566 to 4d7340b Compare December 6, 2023 06:55

JustinBeBoy requested changes Dec 6, 2023

View reviewed changes

JoeGruffins force-pushed the addmixerscreens branch 2 times, most recently from 5aa3fff to c8d5703 Compare December 8, 2023 04:16

JustinBeBoy previously requested changes Dec 8, 2023

View reviewed changes

dcr: Use default account on creation.

064e73e

In order to make mixing opt in, start with it off and do not create the accounts. Allow sending from the default account on creation.

JoeGruffins force-pushed the addmixerscreens branch 2 times, most recently from 3822f66 to da8ba79 Compare December 11, 2023 08:38

dreacot approved these changes Dec 11, 2023

View reviewed changes

ui/page/privacy/manual_mixer_setup_page.go Outdated Show resolved Hide resolved

dreacot reviewed Dec 11, 2023

View reviewed changes

ui/page/privacy/manual_mixer_setup_page.go Outdated Show resolved Hide resolved

dcr: Reimplemint privacy screens.

ec869d0

JoeGruffins force-pushed the addmixerscreens branch from da8ba79 to ec869d0 Compare December 12, 2023 03:02

dreacot approved these changes Dec 12, 2023

View reviewed changes

dreacot merged commit 6c0785e into crypto-power:master Dec 12, 2023
1 check passed

		return layout.Flex{Axis: layout.Vertical, Alignment: layout.Start}.Layout(gtx,
		layout.Rigid(func(gtx C) D {

dcr: Reimplemint privacy screens. #248

dcr: Reimplemint privacy screens. #248

Conversation

JoeGruffins commented Nov 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoeGruffins commented Nov 13, 2023

JoeGruffins left a comment • edited Loading

Choose a reason for hiding this comment

JoeGruffins commented Nov 14, 2023

JoeGruffins commented Nov 14, 2023

dreacot commented Nov 14, 2023 • edited Loading

JoeGruffins commented Nov 15, 2023 • edited Loading

JoeGruffins commented Nov 15, 2023

Sirmorrison 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dreacot commented Nov 22, 2023 • edited Loading

dreacot commented Nov 22, 2023

dreacot commented Nov 22, 2023

JoeGruffins commented Nov 24, 2023

JoeGruffins commented Nov 24, 2023 • edited Loading

JoeGruffins commented Nov 24, 2023

dreacot commented Nov 29, 2023 • edited Loading

dreacot commented Nov 29, 2023

JoeGruffins commented Nov 30, 2023

dreacot commented Nov 30, 2023

JoeGruffins commented Nov 30, 2023

dreacot commented Nov 30, 2023

JoeGruffins commented Nov 30, 2023

dreacot commented Dec 5, 2023

JoeGruffins commented Dec 6, 2023

JustinBeBoy 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

JoeGruffins commented Dec 7, 2023

dreacot commented Dec 7, 2023

JoeGruffins commented Dec 8, 2023

JustinBeBoy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoeGruffins commented Dec 11, 2023

JoeGruffins commented Dec 12, 2023

JoeGruffins left a comment •

edited

Loading

dreacot commented Nov 14, 2023 •

edited

Loading

JoeGruffins commented Nov 15, 2023 •

edited

Loading

dreacot commented Nov 22, 2023 •

edited

Loading

JoeGruffins commented Nov 24, 2023 •

edited

Loading

dreacot commented Nov 29, 2023 •

edited

Loading