This PR addresses an issue where, with no wallet selected, ticking on “Settings -> Mask values” checkbox twice enables the transaction tab when the checkbox is unticked.
Note for maintaners: this PR should be backported to both 25.x and 26.x.
Originally this PR was disabling the “Mask Values” checkbox when no wallet was selected but since a reviewer pointed out that a user might want to open a wallet already on “privacy mode” I rolled that change out.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | alfonsoromanz, hebasto |
| Concept ACK | luke-jr, BrandonOdiwuor |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
nice find
I’m not sure if this is the right approach: user might want to enable Mask values before opening his wallet?
I’m not sure if this is the right approach: user might want to enable
Mask valuesbefore opening his wallet?
Interesting point of view, I agree with it… Not sure if other wallet apps do it but ok, I’ll remove that “fix” and perhaps other reviewers give their opinions on it. Thanks for the review!
The Transaction View should be only enabled when a wallet is selected.
Therefore it has been added a condition for a selected wallet on
enableHistoryAction() since its availability also depends on the mask
value checkbox.
I’m not sure if this is the right approach: user might want to enable
Mask valuesbefore opening his wallet?
I was just giving it a second thought and if we follow the original approach a user could still have the desired behaviour as the following:
First time the user opens or creates a wallet can tick the checkbox on “Mask Values”, this is also persisted, so next time the wallet is open or selected will be on privacy mode. The fix was not changing the current value of the “Mask Values”, so if a user updates QT to the version containing the fix it won’t impact on their setup.
I’m not sure if this is the right approach: user might want to enable
Mask valuesbefore opening his wallet?I was just giving it a second thought and if we follow the original approach a user could still have the desired behaviour as the following: check display here
First time the user opens or creates a wallet can tick the checkbox on “Mask Values”, this is also persisted, so next time the wallet is open or selected will be on privacy mode. The fix was not changing the current value of the “Mask Values”, so if a user updates QT to the version containing the fix it won’t impact on their setup. @pablomartin4btc yes correct it remembers if
Mask Valuesis enabled or not.
Being only able to change it requires a wallet to be open, to me would feel like a bug, no? that’s more my “concern”
Being only able to change it requires a wallet to be open, to me would feel like a bug, no? that’s more my “concern”
Fair enough, thanks for sharing your insights.
