Skip to content

Conversation

@UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Dec 4, 2025

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:

  • 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
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@UdjinM6 UdjinM6 added this to the 23.1 milestone Dec 4, 2025
@github-actions
Copy link

github-actions bot commented Dec 4, 2025

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

@coderabbitai
Copy link

coderabbitai bot commented Dec 4, 2025

Walkthrough

The 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

  • Verify dark.css and light.css contain the intended border/background color rules and that removed verbose rules are not referenced elsewhere.
  • Check general.css for accidental selector duplication or changed specificity after moving rules out of the Layout subsection.
  • Render the dialog in both themes to confirm visual parity and no regressions (focus states, label/checkbox colors, and line edit styles).

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: refactoring MnemonicVerificationDialog CSS by moving it and removing excessive styling rules.
Description check ✅ Passed The description is related to the changeset, explaining the motivation (moving CSS to correct location and cleaning it up) and testing approach, though it lacks specific details about what was changed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 25e17105c4e6823787329f799fece6de6ec0b3fe and 85ae34a.

📒 Files selected for processing (3)
  • src/qt/res/css/dark.css (1 hunks)
  • src/qt/res/css/general.css (1 hunks)
  • src/qt/res/css/light.css (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/qt/res/css/general.css
  • src/qt/res/css/dark.css
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.
⏰ 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)
  • GitHub Check: linux64_ubsan-build / Build source
  • GitHub Check: linux64-build / Build source
  • GitHub Check: linux64_fuzz-build / Build source
  • GitHub Check: linux64_sqlite-build / Build source
  • GitHub Check: linux64_nowallet-build / Build source
  • GitHub Check: linux64_tsan-build / Build source
  • GitHub Check: arm-linux-build / Build source
  • GitHub Check: mac-build / Build source
  • GitHub Check: win64-build / Build source
  • GitHub Check: Lint / Run linters

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Dec 4, 2025

This pull request has conflicts, please rebase.

@github-actions
Copy link

github-actions bot commented Dec 4, 2025

This pull request has conflicts, please rebase.

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK 85ae34a

@thephez
Copy link
Collaborator

thephez commented Dec 22, 2025

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).

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>           - ???

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants