Skip to content

Persist Mask Values option #701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 9, 2023
Merged

Persist Mask Values option #701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/qt/bitcoingui.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,8 @@ void BitcoinGUI::setClientModel(ClientModel *_clientModel, interfaces::BlockAndH
// initialize the disable state of the tray icon with the current value in the model.
trayIcon->setVisible(optionsModel->getShowTrayIcon());
}

m_mask_values_action->setChecked(_clientModel->getOptionsModel()->getOption(OptionsModel::OptionID::MaskValues).toBool());
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
m_mask_values_action->setChecked(_clientModel->getOptionsModel()->getOption(OptionsModel::OptionID::MaskValues).toBool());
m_mask_values_action->setChecked(optionsModel->getMaskValues());

accompanied with

--- a/src/qt/optionsmodel.h
+++ b/src/qt/optionsmodel.h
@@ -98,6 +98,7 @@ public:
     bool getSubFeeFromAmount() const { return m_sub_fee_from_amount; }
     bool getEnablePSBTControls() const { return m_enable_psbt_controls; }
     const QString& getOverriddenByCommandLine() { return strOverriddenByCommandLine; }
+    bool getMaskValues() const { return m_mask_values; }
 
     /* Explicit setters */
     void SetPruneTargetGB(int prune_target_gb);

Copy link
Contributor

Choose a reason for hiding this comment

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

I think @ryanofsky was recommending to remove the case from the SettingsName due to its purpose of mapping OptionsModel to command line options but to add OptionsModel::MaksValues (or leave in this case as @achow101 already added them) to setOption and getOption.

The implementation of the getter looks simpler but then you would need to add the setter too to make it consistent and revert the above (remove the case from setOption and getOption).

Perhaps as a follow-up we could check the model consistent with either solution for the rest of settings (?).

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I'll leave this as is since it's consistent with most of the other settings which use the generic function with the enum.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, you could just use OptionsModel::MaskValues without the OptionID, is this an issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not an issue, will change if I need to retouch.

} else {
if(trayIconMenu)
{
Expand Down
8 changes: 8 additions & 0 deletions src/qt/optionsmodel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ bool OptionsModel::Init(bilingual_str& error)
m_use_embedded_monospaced_font = settings.value("UseEmbeddedMonospacedFont").toBool();
Q_EMIT useEmbeddedMonospacedFontChanged(m_use_embedded_monospaced_font);

m_mask_values = settings.value("mask_values", false).toBool();

return true;
}

Expand Down Expand Up @@ -435,6 +437,8 @@ QVariant OptionsModel::getOption(OptionID option) const
return SettingToBool(setting(), DEFAULT_LISTEN);
case Server:
return SettingToBool(setting(), false);
case MaskValues:
return m_mask_values;
default:
return QVariant();
}
Expand Down Expand Up @@ -612,6 +616,10 @@ bool OptionsModel::setOption(OptionID option, const QVariant& value)
setRestartRequired(true);
}
break;
case MaskValues:
m_mask_values = value.toBool();
settings.setValue("mask_values", m_mask_values);
break;
default:
break;
}
Expand Down
2 changes: 2 additions & 0 deletions src/qt/optionsmodel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ class OptionsModel : public QAbstractListModel
Listen, // bool
Server, // bool
EnablePSBTControls, // bool
MaskValues, // bool
OptionIDRowCount,
};

Expand Down Expand Up @@ -120,6 +121,7 @@ class OptionsModel : public QAbstractListModel
bool fCoinControlFeatures;
bool m_sub_fee_from_amount;
bool m_enable_psbt_controls;
bool m_mask_values;

//! In-memory settings for display. These are stored persistently by the
//! bitcoin node but it's also nice to store them in memory to prevent them
Expand Down
1 change: 1 addition & 0 deletions src/qt/overviewpage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ void OverviewPage::handleTransactionClicked(const QModelIndex &index)
void OverviewPage::setPrivacy(bool privacy)
{
m_privacy = privacy;
clientModel->getOptionsModel()->setOption(OptionsModel::OptionID::MaskValues, privacy);
const auto& balances = walletModel->getCachedBalance();
if (balances.balance != -1) {
setBalance(balances);
Expand Down