feat(qt): persist filter preferences in masternode list#7148
feat(qt): persist filter preferences in masternode list#7148PastaPastaPasta merged 2 commits intodashpay:developfrom
Conversation
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
WalkthroughThe changes add persistent storage of MasternodeList UI filter preferences using QSettings. The MasternodeList constructor loads saved settings (HideBanned, TypeFilter, FilterText, OwnedOnly) and applies them to the UI and proxy model on startup. Handlers for filter text, type filter, hide-banned, and owned-only now save updates to QSettings. The default m_hide_banned value (and the "Hide banned" checkbox default) is changed from true to false. Sequence Diagram(s)sequenceDiagram
participant User
participant UI as MasternodeList UI
participant Model as MasternodeListSortFilterProxyModel
participant Settings as QSettings
participant Wallet as WalletModel
User->>UI: Open Masternode list
UI->>Settings: read(HideBanned, TypeFilter, FilterText, OwnedOnly)
Settings-->>UI: return saved values
UI->>Model: apply filters (HideBanned, TypeFilter, FilterText, OwnedOnly)
UI->>UI: update checkbox/text controls
alt WalletModel present
Wallet->>UI: setWalletModel()
UI->>Settings: read(OwnedOnly)
Settings-->>UI: return OwnedOnly
UI->>Model: apply OwnedOnly
end
User->>UI: change filter controls
UI->>Model: update filter
UI->>Settings: write(updated filter values)
Settings-->>UI: confirm write
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/qt/masternodelist.cpp (1)
336-338: Consider debouncing the QSettings write for filter text.
on_filterText_textChangedfires on every keystroke, creating aQSettingsinstance and writing each time. While Qt typically batches filesystem writes, this is still unnecessary churn. The other filters (checkbox/combobox) change infrequently, so they're fine — but text input can generate rapid-fire calls.A lightweight alternative: save the filter text in the destructor or on a short timer, rather than on every character.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/qt/masternodelist.cpp (3)
143-148: Consider defining settings key names as constants to avoid duplication.The same string literals (
"mnListHideBanned","mnListTypeFilter","mnListFilterText","mnListOwnedOnly") appear in both the load (here) and save (in each handler) paths. Extracting them toconstexpr/constvariables in the anonymous namespace would prevent silent typo-induced bugs.Example
namespace { constexpr int MASTERNODELIST_UPDATE_SECONDS{3}; const QString SETTINGS_KEY_HIDE_BANNED{"mnListHideBanned"}; const QString SETTINGS_KEY_TYPE_FILTER{"mnListTypeFilter"}; const QString SETTINGS_KEY_FILTER_TEXT{"mnListFilterText"}; const QString SETTINGS_KEY_OWNED_ONLY{"mnListOwnedOnly"}; } // anonymous namespace
147-147: Clamp the persisted type-filter index before applying.
QComboBox::setCurrentIndexsilently accepts out-of-range values (sets selection to -1). While the downstream slot guards against negative/overflow indices, loading a corrupted setting would leave the combo box with no visible selection. A quick clamp at load time is more defensive.Proposed fix
- ui->comboBoxType->setCurrentIndex(settings.value("mnListTypeFilter", 0).toInt()); + const int type_filter = settings.value("mnListTypeFilter", 0).toInt(); + if (type_filter >= 0 && type_filter < static_cast<int>(MasternodeListSortFilterProxyModel::TypeFilter::COUNT)) { + ui->comboBoxType->setCurrentIndex(type_filter); + }
336-338: Filter text is saved on every keystroke.
on_filterText_textChangedfires per character. This is fine forQSettings(writes are cheap and batched on most platforms), but if this ever becomes a concern, debouncing the save — or saving only on focus-out / window close — would be an option.
…e list a653b26 feat(qt): persist filter preferences in masternode list (Kittywhiskers Van Gogh) 6c97bc1 qt: don't hide banned nodes by default (Kittywhiskers Van Gogh) Pull request description: ## Additional Information In response to community feedback to the UI changes done in [dash#7116](dashpay#7116), the following changes have been implemented * Banned masternodes are no longer hidden by default (to aid MNOs in knowing the status of their nodes at immediate glance) * Persisting filter preferences across restarts (the new layout introduced four different filter options and they'll now be remembered across restarts) ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)** - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: light ACK a653b26 Tree-SHA512: 471f2d174e64396c22ad8c0ed2c0cc50d85bf0d2b63cd1a85feb720358d78a78c14bc1065ca30a762badd7bea35cbefd21f9105e66e0443ae923abfcaccc2a95
00f590d Merge #7180: qt: add Tahoe styled icons for macOS, runtime styling for each network type, update bundle icon, add mask-based tray icon, generation scripts (pasta) 60dda51 Merge #7176: perf: do linear lookup instead building 2 heavy Hash-Maps (pasta) df1ca87 Merge #7159: feat(qt): UI refresh (5/n, add proposal information widget to information, donut chart for proposal allocation) (pasta) 9061ad0 Merge #7118: feat(qt): UI refresh (4/n, introduce distinct widgets for Dash-specific reporting in debug window) (pasta) 64cc4f2 Merge #7160: feat(interfaces): consolidate masternode counts into one struct, expose chainlock, instantsend, credit pool, quorum statistics (pasta) 5d28a69 Merge #7157: fix(qt): prevent banned masternodes from returning status=0 (pasta) e0b7386 Merge #7146: feat(qt): introduce framework for sourcing and applying data, use for `{Masternode,Proposal}List`s (pasta) 8fd53cd Merge #7144: feat(qt): add support for reporting `OP_RETURN` payloads as Data Transactions (pasta) cc6f0bb Merge #7154: fix: MN update notifications had old_list/new_list swapped (pasta) 33f0138 Merge #7145: fix(qt): move labelError styling from proposalcreate.ui into general.css (pasta) 1bdbde6 Merge #7148: feat(qt): persist filter preferences in masternode list (pasta) 96bb601 Merge #7147: fix(qt): prevent overview page font double scaling, recalculate minimum width correctly, `SERVICE` and `STATUS` sorting, fix common types filtering (pasta) Pull request description: ## Backport PRs for v23.1.1 Cherry-picks the following 12 PRs (labeled `backport-candidate-23.1.x`) from `develop` onto `v23.1.x`, in merge order: | PR | Title | |---|---| | #7147 | fix(qt): prevent overview page font double scaling, recalculate minimum width correctly, `SERVICE` and `STATUS` sorting, fix common types filtering | | #7148 | feat(qt): persist filter preferences in masternode list | | #7145 | fix(qt): move labelError styling from proposalcreate.ui into general.css | | #7154 | fix: MN update notifications had old_list/new_list swapped | | #7144 | feat(qt): add support for reporting `OP_RETURN` payloads as Data Transactions | | #7146 | feat(qt): introduce framework for sourcing and applying data, use for `{Masternode,Proposal}List`s | | #7157 | fix(qt): prevent banned masternodes from returning status=0 | | #7160 | feat(interfaces): consolidate masternode counts into one struct, expose chainlock, instantsend, credit pool, quorum statistics | | #7118 | feat(qt): UI refresh (4/n, introduce distinct widgets for Dash-specific reporting in debug window) | | #7159 | feat(qt): UI refresh (5/n, add proposal information widget to information, donut chart for proposal allocation) | | #7176 | perf: do linear lookup instead building 2 heavy Hash-Maps | | #7180 | qt: add Tahoe styled icons for macOS, runtime styling for each network type, update bundle icon, add mask-based tray icon, generation scripts | All 12 cherry-picked cleanly (no conflicts). ## Notes - Used `git cherry-pick -m 1 <merge-sha>` for each (all were merge commits on develop) - Applied in chronological merge order to respect dependency chains - Version bump and release notes are separate steps per the release process ACKs for top commit: kwvg: utACK 00f590d UdjinM6: utACK 00f590d Tree-SHA512: 90d2a0660db8daa69b3e3b33a8a790fb0ea7d9a04656a2e27955575e76b6f4c9a379c435ef1c573ef6669c36cb6e205ba9701716d3dc303b01f19c719516b6d1
Additional Information
In response to community feedback to the UI changes done in dash#7116, the following changes have been implemented
Breaking Changes
None expected.
Checklist