Skip to content

fix: use theme colors for button icons#949

Merged
nogeenharrie merged 1 commit intomainfrom
fix/theme-aware-button-icons
Apr 9, 2026
Merged

fix: use theme colors for button icons#949
nogeenharrie merged 1 commit intomainfrom
fix/theme-aware-button-icons

Conversation

@nogeenharrie
Copy link
Copy Markdown
Contributor

@nogeenharrie nogeenharrie commented Apr 8, 2026

User description

Summary

Add setForegroundRole(QPalette::ButtonText) to make eye, QR code, and clipboard icons theme-aware instead of hardcoded black in dark mode.

Fixes icons appearing black in dark mode for:

  • Password show/hide button (eye)
  • QR code button
  • Copy to clipboard button

Summary by CodeRabbit

  • Bug Fixes

    • Button foreground now correctly follows the app palette for QR, show-password, and clipboard buttons.
    • Show-password button uses standard visibility/visibility-off theme icons (with existing fallbacks preserved).
  • Style

    • Enforced consistent icon sizing and inherited color for these buttons to improve visual consistency.

CodeAnt-AI Description

Use theme colors for action icons in dark mode

What Changed

  • The password show/hide button, QR code button, and copy button now use the app’s theme color instead of staying black
  • These icons are readable in dark mode and match the rest of the button text color
  • The password visibility button also keeps its icon sizing and spacing while following the theme color

Impact

✅ Clearer dark mode icons
✅ Fewer unreadable action buttons
✅ More consistent theme appearance

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai bot commented Apr 8, 2026

CodeAnt AI is reviewing your PR.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 8, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Three QPushButton-derived classes now set their foreground role to QPalette::ButtonText after initializing their icons. The show-password button also swaps icon theme keys to "visibility" / "visibility-off". MainWindow::addToGridLayout updates button stylesheets to include icon-size: 16px and color: inherit.

Changes

Cohort / File(s) Summary
Foreground Role Configuration
src/qpushbuttonasqrcode.cpp, src/qpushbuttonwithclipboard.cpp, src/qpushbuttonshowpassword.cpp
Added setForegroundRole(QPalette::ButtonText) in each button constructor after setting the icon.
Show/Hide Icon Theme Keys
src/qpushbuttonshowpassword.cpp
Replaced theme keys passed to QIcon::fromTheme from "show"/"hide-new" to "visibility"/"visibility-off"; SVG fallbacks and runtime toggle logic unchanged.
Button Stylesheet Updates
src/mainwindow.cpp
Extended setStyleSheet(...) for clipboard, QR, and show-password buttons to add icon-size: 16px; and color: inherit; alongside existing rules.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐇 I hopped through buttons, small and bright,
Set text to gleam and icons to light,
Swapped a key, fixed a hue,
Styles aligned — a rabbit’s view! 🥕✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: use theme colors for button icons' directly matches the main change: setting foreground palette roles and icon theme keys to make button icons use theme colors instead of hardcoded values.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/theme-aware-button-icons

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

@codeant-ai codeant-ai bot added the size:XS This PR changes 0-9 lines, ignoring generated files label Apr 8, 2026
@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai bot commented Apr 8, 2026

CodeAnt AI finished reviewing your PR.

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 8, 2026

Coverage Status

coverage: 21.056% (+0.06%) from 21.0% — fix/theme-aware-button-icons into main

@annejan
Copy link
Copy Markdown
Member

annejan commented Apr 8, 2026

It's still black . .

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 27.20%. Comparing base (0b54f81) to head (a991624).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #949      +/-   ##
==========================================
+ Coverage   27.13%   27.20%   +0.06%     
==========================================
  Files          39       39              
  Lines        3258     3261       +3     
==========================================
+ Hits          884      887       +3     
  Misses       2374     2374              
Flag Coverage Δ
qtpass 27.20% <100.00%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nogeenharrie nogeenharrie force-pushed the fix/theme-aware-button-icons branch from a991624 to 7463833 Compare April 9, 2026 10:36
@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai bot commented Apr 9, 2026

CodeAnt AI is running Incremental review

@codeant-ai codeant-ai bot added size:XS This PR changes 0-9 lines, ignoring generated files and removed size:XS This PR changes 0-9 lines, ignoring generated files labels Apr 9, 2026
@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai bot commented Apr 9, 2026

CodeAnt AI Incremental review completed.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/mainwindow.cpp`:
- Around line 1275-1276: The QR and copy button style strings are missing
"color: inherit" which makes them stay black in dark mode; update the style
declarations for the QR button and the copy button to include "color: inherit"
(matching the show-password button's style string "border-style: none;
background: transparent; padding: 0; margin: 0; icon-size: 16px; color:
inherit;") so all three buttons inherit text color for dark-mode support.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 406e2b29-125e-4816-a8c5-36d89d440922

📥 Commits

Reviewing files that changed from the base of the PR and between a991624 and 7463833.

📒 Files selected for processing (4)
  • src/mainwindow.cpp
  • src/qpushbuttonasqrcode.cpp
  • src/qpushbuttonshowpassword.cpp
  • src/qpushbuttonwithclipboard.cpp

@nogeenharrie nogeenharrie force-pushed the fix/theme-aware-button-icons branch from 7463833 to e37011b Compare April 9, 2026 10:47
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
src/mainwindow.cpp (1)

1275-1276: ⚠️ Potential issue | 🟠 Major

Apply the same inherited color styling to copy/QR buttons too.

At Line 1275 only the show-password button gets color: inherit; the copy button (Line 1245) and QR button (Line 1254) still use the old style string, which explains the “still black” behavior in dark themes.

Proposed fix
-    fieldLabel->setStyleSheet(
-        "border-style: none ; background: transparent; padding: 0; margin: 0;");
+    fieldLabel->setStyleSheet(
+        "border-style: none; background: transparent; padding: 0; margin: 0; "
+        "icon-size: 16px; color: inherit;");

@@
-    qrbutton->setStyleSheet(
-        "border-style: none ; background: transparent; padding: 0; margin: 0;");
+    qrbutton->setStyleSheet(
+        "border-style: none; background: transparent; padding: 0; margin: 0; "
+        "icon-size: 16px; color: inherit;");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/mainwindow.cpp` around lines 1275 - 1276, The copy and QR button style
strings need to match the show-password button so they inherit theme colors;
update the style assignment used for the copy button and the QR button to
include "color: inherit" (and keep the existing "border-style: none; background:
transparent; padding: 0; margin: 0; icon-size: 16px;") so their style strings
mirror the show-password button's style and stop rendering black in dark themes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/mainwindow.cpp`:
- Around line 1275-1276: The copy and QR button style strings need to match the
show-password button so they inherit theme colors; update the style assignment
used for the copy button and the QR button to include "color: inherit" (and keep
the existing "border-style: none; background: transparent; padding: 0; margin:
0; icon-size: 16px;") so their style strings mirror the show-password button's
style and stop rendering black in dark themes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: c6b1a73f-de31-41bc-9108-82cd5e45e70f

📥 Commits

Reviewing files that changed from the base of the PR and between 7463833 and e37011b.

📒 Files selected for processing (4)
  • src/mainwindow.cpp
  • src/qpushbuttonasqrcode.cpp
  • src/qpushbuttonshowpassword.cpp
  • src/qpushbuttonwithclipboard.cpp

@nogeenharrie nogeenharrie force-pushed the fix/theme-aware-button-icons branch from e37011b to 733c7d2 Compare April 9, 2026 10:53
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/qpushbuttonshowpassword.cpp (1)

17-23: ⚠️ Potential issue | 🟠 Major

Show-password button is missing explicit ButtonText foreground role.

The constructor in src/qpushbuttonshowpassword.cpp never calls setForegroundRole(QPalette::ButtonText), while both peer button constructors (QPushButtonAsQRCode and QPushButtonWithClipboard) include this call. This inconsistency can leave the icon rendering with incorrect colors in dark themes, defeating the PR's theme-aware icon intent.

Proposed fix
 QIcon::fromTheme("visibility-off", QIcon(":/icons/hide.svg"))) {
   setIcon(iconEdit);
+  setForegroundRole(QPalette::ButtonText);
   connect(this, &QPushButton::clicked, this,
           &QPushButtonShowPassword::buttonClicked);
   this->line = line;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/qpushbuttonshowpassword.cpp` around lines 17 - 23, The
QPushButtonShowPassword constructor is missing a call to set the palette role
for icon coloring: add a call to setForegroundRole(QPalette::ButtonText) inside
the QPushButtonShowPassword constructor (the same place where iconEdit and
iconEditPushed are set and connect is called) so the button's icon uses the
ButtonText role in dark themes (consistent with QPushButtonAsQRCode and
QPushButtonWithClipboard).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/qpushbuttonshowpassword.cpp`:
- Around line 17-23: The QPushButtonShowPassword constructor is missing a call
to set the palette role for icon coloring: add a call to
setForegroundRole(QPalette::ButtonText) inside the QPushButtonShowPassword
constructor (the same place where iconEdit and iconEditPushed are set and
connect is called) so the button's icon uses the ButtonText role in dark themes
(consistent with QPushButtonAsQRCode and QPushButtonWithClipboard).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: b96009f5-3e8f-4f81-b9ee-11ec81dd1553

📥 Commits

Reviewing files that changed from the base of the PR and between e37011b and 733c7d2.

📒 Files selected for processing (4)
  • src/mainwindow.cpp
  • src/qpushbuttonasqrcode.cpp
  • src/qpushbuttonshowpassword.cpp
  • src/qpushbuttonwithclipboard.cpp

@nogeenharrie nogeenharrie force-pushed the fix/theme-aware-button-icons branch from 733c7d2 to 5912503 Compare April 9, 2026 11:08
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/mainwindow.cpp`:
- Around line 1245-1246: Extract the duplicated inline stylesheet string into a
single constant (e.g., kButtonStyle or BUTTON_STYLESHEET) and replace the three
identical literal occurrences with that constant; update the usages where the
string is passed to the button setup (the places around the existing inline
strings and the other occurrences noted at the same pattern at lines referenced
in the review) so all buttons use the single constant to avoid drift between
"border-style: none; background: transparent; padding: 0; margin: 0; icon-size:
16px; color: inherit;". Ensure the constant is defined in the same translation
unit/scope that covers the callers so no public API changes are required.

In `@src/qpushbuttonshowpassword.cpp`:
- Around line 17-19: Replace the non-standard icon names used when constructing
iconEdit and iconEditPushed so themes can provide proper icons: change
QIcon::fromTheme("visibility", ...) to use a verified name such as
"password-show-on" with a fallback like "view-reveal-symbolic", and change
QIcon::fromTheme("visibility-off", ...) to "password-show-off" with a fallback
like "view-conceal-symbolic"; update the iconEdit and iconEditPushed
initializers to call QIcon::fromTheme with the chosen verified name and keep the
existing embedded SVG fallback unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0a35cc33-833d-466b-83f7-11e86d175091

📥 Commits

Reviewing files that changed from the base of the PR and between 733c7d2 and 5912503.

📒 Files selected for processing (4)
  • src/mainwindow.cpp
  • src/qpushbuttonasqrcode.cpp
  • src/qpushbuttonshowpassword.cpp
  • src/qpushbuttonwithclipboard.cpp

@nogeenharrie nogeenharrie force-pushed the fix/theme-aware-button-icons branch from 5912503 to 10fde45 Compare April 9, 2026 11:18
Use a single buttonStyle constant instead of duplicated inline strings.
@nogeenharrie nogeenharrie force-pushed the fix/theme-aware-button-icons branch from 10fde45 to e928465 Compare April 9, 2026 11:21
@nogeenharrie nogeenharrie merged commit b2577f8 into main Apr 9, 2026
23 of 24 checks passed
@nogeenharrie nogeenharrie deleted the fix/theme-aware-button-icons branch April 9, 2026 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS This PR changes 0-9 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants