Skip to content

Conversation

@UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Dec 4, 2025

Issue being fixed or feature implemented

Should look like this now

Screenshot 2025-12-04 at 03 25 06 Screenshot 2025-12-04 at 03 25 11

I also dropped some redundant styling while at it. And the dialog should now also shrink to its original size when advanced options are hidden.

What was done?

See commits

How Has This Been Tested?

Run, check the dialog with dark/light theme while switching os themes too.

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
@UdjinM6 UdjinM6 requested review from knst and thephez December 4, 2025 00:49
@github-actions
Copy link

github-actions bot commented Dec 4, 2025

⚠️ Potential Merge Conflicts Detected

This PR has potential conflicts with the following open PRs:

Please coordinate with the authors of these PRs to avoid merge conflicts.

@coderabbitai
Copy link

coderabbitai bot commented Dec 4, 2025

Walkthrough

The PR changes the Create Wallet dialog UI and styling and adjusts toggle behavior. The dialog now enforces a fixed size. The advanced options toggle was converted from an icon/arrow to a text toggle ("Show Advanced Options" / "Hide Advanced Options") and its placement was moved into a new horizontal layout alongside the dialog button box. Theme CSS was reorganized: CreateWalletDialog rules were consolidated into general.css and button selectors were added/adjusted across dark and light theme files.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • src/qt/createwalletdialog.cpp: confirm fixed-size constraint and new text-based toggle updates visibility/state correctly.
  • src/qt/forms/createwalletdialog.ui: verify new horizontalLayout2, placement of advanced_toggle_button and buttonBox, and accessibility/focus behavior.
  • src/qt/res/css/{general.css,dark.css,light.css}: check consolidated selectors for CreateWalletDialog and ensure styling parity across themes.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: improve CreateWalletDialog look' clearly and concisely summarizes the main objective of the PR - improving the visual appearance of the CreateWalletDialog. It is specific, directly related to the changeset, and appropriately highlights the primary change.
Description check ✅ Passed The description is directly related to the changeset, explaining the visual improvements to CreateWalletDialog, mentioning redundant styling removal, the shrink-to-size behavior, and providing testing details. Though the checklist items are unchecked, the description clearly documents what was done and how it was tested.
✨ 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 39e2f2b and c34bd8b.

📒 Files selected for processing (4)
  • src/qt/forms/createwalletdialog.ui (1 hunks)
  • src/qt/res/css/dark.css (4 hunks)
  • src/qt/res/css/general.css (2 hunks)
  • src/qt/res/css/light.css (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/qt/forms/createwalletdialog.ui
  • src/qt/res/css/general.css
  • src/qt/res/css/dark.css
⏰ 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). (9)
  • GitHub Check: arm-linux-build / Build source
  • GitHub Check: linux64_nowallet-build / Build source
  • GitHub Check: linux64_tsan-build / Build source
  • GitHub Check: mac-build / Build source
  • GitHub Check: win64-build / Build source
  • GitHub Check: linux64_ubsan-build / Build source
  • GitHub Check: linux64_sqlite-build / Build source
  • GitHub Check: linux64-build / Build source
  • GitHub Check: linux64_fuzz-build / Build source
🔇 Additional comments (2)
src/qt/res/css/light.css (2)

382-382: LGTM!

The QDialog#CreateWalletDialog QToolButton#advanced_toggle_button selector is correctly integrated into the light theme button styling hierarchy, covering all necessary pseudo-states (:hover, :pressed, :disabled). The styling is consistent with other light-themed dialog buttons in the codebase.

Also applies to: 400-400, 418-418, 436-436


271-271: Verify that the QGroupBox background change applies only to intended elements.

The background color change from #f2f2f4 to #eaeaec affects all QGroupBox elements in the light theme, not just those in CreateWalletDialog. Confirm this broad change is intentional and doesn't inadvertently darken other dialogs or pages that use QGroupBox.


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.

@knst
Copy link
Collaborator

knst commented Dec 4, 2025

with light frame gap looks a bit big:
image
but it's much better than it used to be!

<bool>true</bool>
</property>
<property name="focusPolicy">
<enum>Qt::NoFocus</enum>
Copy link
Collaborator

Choose a reason for hiding this comment

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

should remove this one. Otherwise can't get to this button with 'Tab', it requires mouse to press it.

@UdjinM6
Copy link
Author

UdjinM6 commented Dec 4, 2025

With light theme it should look like this now
Screenshot 2025-12-04 at 13 09 54

Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

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

ACK c34bd8b

Copy link
Collaborator

@thephez thephez left a comment

Choose a reason for hiding this comment

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

Lightly-tested ACK c34bd8b

@PastaPastaPasta PastaPastaPasta merged commit f5f9fd3 into dashpay:develop Dec 4, 2025
33 of 36 checks passed
@github-actions
Copy link

github-actions bot commented Dec 4, 2025

This pull request has conflicts, please rebase.

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.

4 participants