-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Several ui fixes #11967
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Several ui fixes #11967
Conversation
Can you add the font size reset on theme change fix? |
keepassxc/src/gui/MainWindow.cpp Lines 1991 to 1997 in a6e92c9
I think that it can be fixed like this
|
56ca23e
to
7d7476d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR centralizes UI styling fixes by migrating splitter handle colors into code, refines various widget styles, and ensures dynamic font sizing with theme changes.
- Migrate splitter handle styling from dark QSS into
BaseStyle
code and fix its contrast. - Update error highlighting in
ApplicationSettingsWidget
to useStateColorPalette
, align a settings label, and remove an obsolete tab stop. - Apply font size changes immediately when the theme is toggled in
MainWindow
.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/gui/styles/dark/darkstyle.qss | Removed QSplitterHandle rule as splitter styling is now handled in code |
src/gui/styles/base/BaseStyle.cpp | Added S_splitterHandle color entry and updated drawControl splitter logic |
src/gui/dbsettings/DatabaseSettingsWidgetEncryption.ui | Removed advancedSettingsButton from the tab order |
src/gui/MainWindow.cpp | Invoke applyFontSize() after theme application |
src/gui/ApplicationSettingsWidgetGeneral.ui | Right-align "Remember last typed entry for:" label |
src/gui/ApplicationSettingsWidget.cpp | Replace hardcoded error CSS with StateColorPalette for shortcut widget |
Comments suppressed due to low confidence (2)
src/gui/styles/dark/darkstyle.qss:32
- [nitpick] With splitter styling now in code, consider adding a comment or consolidating other QSS selectors (e.g., QSplitter::handle) to clarify where and how splitter handles are styled.
QSplitterHandle {
src/gui/MainWindow.cpp:1997
- [nitpick] Consider adding a unit or integration test to verify that font size changes are applied correctly when the view menu setting is toggled.
kpxcApp->applyFontSize();
painter->fillRect(filledRect, swatch.color(S_button_specular)); | ||
Ph::fillRectOutline(painter, filledRect.adjusted(-1, 0, 1, 0), 1, swatch.color(S_window_divider)); | ||
painter->fillRect(filledRect, swatch.color(S_splitterHandle)); | ||
Ph::fillRectOutline(painter, filledRect.adjusted(-1, -1, 1, 1), 1, swatch.color(S_splitterHandle)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Outline is drawn with the same S_splitterHandle
color, making the border invisible. Consider using a contrasting color (e.g., S_window_divider
) for the outline to improve visibility.
Ph::fillRectOutline(painter, filledRect.adjusted(-1, -1, 1, 1), 1, swatch.color(S_splitterHandle)); | |
Ph::fillRectOutline(painter, filledRect.adjusted(-1, -1, 1, 1), 1, swatch.color(S_window_divider)); |
Copilot uses AI. Check for mistakes.
m_generalUi->autoTypeShortcutWidget->setStyleSheet("background-color: #FF9696;"); | ||
StateColorPalette statePalette; | ||
auto color = statePalette.color(StateColorPalette::ColorRole::Error); | ||
m_generalUi->autoTypeShortcutWidget->setStyleSheet(QString("QLineEdit { background: %1; }").arg(color.name())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Use the background-color
property in the style sheet rather than the background
shorthand for clarity, e.g., QLineEdit { background-color: %1; }
.
m_generalUi->autoTypeShortcutWidget->setStyleSheet(QString("QLineEdit { background: %1; }").arg(color.name())); | |
m_generalUi->autoTypeShortcutWidget->setStyleSheet(QString("QLineEdit { background-color: %1; }").arg(color.name())); |
Copilot uses AI. Check for mistakes.
Screenshots
Testing strategy
Manual
Type of change