refactor: move MnemonicVerificationDialog css, drop excessive styling#7040
refactor: move MnemonicVerificationDialog css, drop excessive styling#7040PastaPastaPasta merged 1 commit intodashpay:developfrom
Conversation
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
WalkthroughThe pull request updates MnemonicVerificationDialog CSS across three files. In src/qt/res/css/dark.css and src/qt/res/css/light.css a compact MnemonicVerificationDialog block was added (setting QScrollArea#mnemonicScroll border-color and QWidget#mnemonicGridWidget background-color) and a prior, more verbose theme-specific MnemonicVerificationDialog block was removed. In src/qt/res/css/general.css the MnemonicVerificationDialog rules were relocated from a "Layout" subsection into a top-level MnemonicVerificationDialog block without changing selectors or properties. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📥 CommitsReviewing files that changed from the base of the PR and between 25e17105c4e6823787329f799fece6de6ec0b3fe and 85ae34a. 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
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 |
|
This pull request has conflicts, please rebase. |
33a36b3 to
25e1710
Compare
|
This pull request has conflicts, please rebase. |
25e1710 to
85ae34a
Compare
|
Not sure if it's related to this PR, but I sometimes (not always) get segfaults when switching between themes. Happened at least once going Dark->Light and also Light->Dark (shown below). |
|
ping @knst |
I reproduced issue and it's not related to this PR. I am working on fix. |
…been shown 009104c fix: return full functionality of Back / Cancel button on mnemonic verification dialog (Konstantin Akimov) c24473b fix: crash in mnemonicverificationdialog by proper using reject() event (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented First found by thepez while testing #7040 It happens every time when create new wallet after dialog to validate mnemonic has been shown. Steps to reproduce: 1. Create new wallet 2. Show mnemonic 3. Confirm the mnemonic is saved 4. Close dialog by Canceling validation or by Confirming validation 5. Change theme crashes app ``` 2025-12-22T15:16:31Z Posix Signal: Segmentation fault 0#: (0x608F5BE3FDB5) stl_vector.h:115 - std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data::_M_copy_data(std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data const&) 1#: (0x608F5BE3FDB5) stl_vector.h:127 - std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data::_M_swap_data(std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data&) 2#: (0x608F5BE3FDB5) stl_vector.h:1962 - std::vector<unsigned long, std::allocator<unsigned long> >::_M_move_assign(std::vector<unsigned long, std::allocator<unsigned long> >&&, std::integral_constant<bool, true>) 3#: (0x608F5BE3FDB5) stl_vector.h:771 - std::vector<unsigned long, std::allocator<unsigned long> >::operator=(std::vector<unsigned long, std::allocator<unsigned long> >&&) 4#: (0x608F5BE3FDB5) stacktraces.cpp:784 - HandlePosixSignal 5#: (0x7D6B37A45330) libc_sigaction.c - ??? 6#: (0x608F5CC0951B) <unknown-file> - ??? 7#: (0x608F5CC09901) <unknown-file> - ??? 8#: (0x608F5B62369D) unique_lock.h:105 - std::unique_lock<std::recursive_mutex>::~unique_lock() 9#: (0x608F5B62369D) sync.h:226 - UniqueLock<AnnotatedMixin<std::recursive_mutex> >::~UniqueLock() 10#: (0x608F5B62369D) guiutil.cpp:1031 - GUIUtil::loadStyleSheet(bool) 11#: (0x608F5B6243C4) guiutil.cpp:1616 - GUIUtil::loadTheme(bool) 12#: (0x608F5B6C27B7) atomic_base.h:505 - std::__atomic_base<int>::load(std::memory_order) const 13#: (0x608F5B6C27B7) qatomic_cxx11.h:239 - int QAtomicOps<int>::loadRelaxed<int>(std::atomic<int> const&) 14#: (0x608F5B6C27B7) qbasicatomic.h:107 - QBasicAtomicInteger<int>::loadRelaxed() const 15#: (0x608F5B6C27B7) qrefcount.h:66 - QtPrivate::RefCount::deref() 16#: (0x608F5B6C27B7) qstring.h:1308 - QString::~QString() 17#: (0x608F5B6C27B7) appearancewidget.cpp:110 - AppearanceWidget::updateTheme(QString const&) 18#: (0x608F5C90A0CD) <unknown-file> - ??? 19#: (0x608F5CC5B1E1) <unknown-file> - ??? 20#: (0x608F5CC5D00B) <unknown-file> - ??? 21#: (0x608F5CC5D29B) <unknown-file> - ??? 22#: (0x608F5C90A2EB) <unknown-file> - ??? 23#: (0x608F5CC57376) <unknown-file> - ??? 24#: (0x608F5CC586F6) <unknown-file> - ??? 25#: (0x608F5C8DD693) <unknown-file> - ??? ``` Current call `disconnect(cancel, nullptr, nullptr, nullptr)` is too violent for qt objects because it disconnects not only user-added slots, but internal qt slots too; it makes the object _Cancel Button_ to be in an invalid state. ## What was done? Easiest fix is specify _some_ arguments for disconnect, for example, simple `disconnect(cancel, &QDialogButtonBox::rejected, this, nullptr);` fixes crash but it breaks functionality of Cancel button. For the final solution look to the commits in PR ## How Has This Been Tested? Tested numerous user cases (list is not full): - create wallet, validate mnemonic, click "BACK" - create wallet, close dialog without validation mnemonic - create wallet, validate mnemonic, click BACK, re-validate mnemonic - show mnemonic for already existing wallet Also I tried to do the exactly same things, but instead mouse's click use Escape or Ctrl+W or close window by mouse. I am not sure that I provided 100% coverage by manual testing, but at this point it looks solid for me. Any extra testing is welcome, please help with it! ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only) ACKs for top commit: UdjinM6: ACK 009104c Tree-SHA512: aab64388dcc90b0bd2ab035fc11f0b53c55643dcf3ed6c0859ba2ce3865d8341243892e56fc998391652b9da11be815360c80314c24c6cea90a896196a29f9ae
Issue being fixed or feature implemented
Move it to the right place, also clean it a bit while at it.
What was done?
How Has This Been Tested?
Run, check the dialog with dark/light theme while switching os themes too. Should be no visible changes.
Breaking Changes
n/a
Checklist: