Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add database name and color options for unlock view #10819

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Add database name and color options for unlock view #10819

wants to merge 2 commits into from
+288 −29

Conversation

techninja1008
Copy link

Fixes #10783.

Adds two database configuration options (stored as public custom data) that allow a short summary text to be displayed on the database unlock screen. The user can specify both the message and an optional color which, if set, causes the summary to be displayed on a colored background with an appropriately contrasting text color.

The summary message is stored in KPXC_PUBLIC_SUMMARY and the color is stored in KPXC_PUBLIC_COLOR.

Screenshots

kpxc4
kpxc3
kpxc2
kpxc1

Testing strategy

I have extensively performed manual testing with different values of both the message (empty, short text, long text etc) and color (empty, varying colors, invalid colors etc). I've also tested opening/closing the database, starting keepassxc after it was the last DB opened etc to approach the unlock UI from different code paths.

Type of change

  • ✅ New feature (change that adds functionality)

@droidmonkey
Copy link
Member

droidmonkey commented May 27, 2024 •
edited
Loading

You should use the Qt color picker, see example here:

keepassxc/src/gui/entry/EditEntryWidget.cpp

Lines 1654 to 1687 in 9aa0406

void EditEntryWidget::setupColorButton(bool foreground, const QColor& color)
{
QWidget* button = m_advancedUi->fgColorButton;
QCheckBox* checkBox = m_advancedUi->fgColorCheckBox;
if (!foreground) {
button = m_advancedUi->bgColorButton;
checkBox = m_advancedUi->bgColorCheckBox;
}
if (color.isValid()) {
button->setStyleSheet(QString("background-color:%1").arg(color.name()));
button->setProperty("color", color.name());
checkBox->setChecked(true);
} else {
button->setStyleSheet("");
button->setProperty("color", QVariant());
checkBox->setChecked(false);
}
}
void EditEntryWidget::pickColor()
{
bool isForeground = (sender() == m_advancedUi->fgColorButton);
QColor oldColor = QColor(m_advancedUi->fgColorButton->property("color").toString());
if (!isForeground) {
oldColor = QColor(m_advancedUi->bgColorButton->property("color").toString());
}
QColor newColor = QColorDialog::getColor(oldColor);
if (newColor.isValid()) {
setupColorButton(isForeground, newColor);
setModified(true);
}
}

@droidmonkey droidmonkey added this to the v2.8.0 milestone May 27, 2024
@techninja1008
Copy link
Author

I've added a "Pick color..." button that uses QColorDialog.

image

@techninja1008
Copy link
Author

@droidmonkey let me know if there is anything else you need to make this mergeable.

@droidmonkey
Copy link
Member

Seemed fine on first pass good work

@droidmonkey droidmonkey changed the title Add user-configurable summary to unlock screen. Add database name and color options for unlock view Jun 2, 2024
@droidmonkey
Copy link
Member

droidmonkey commented Jun 2, 2024 •
edited
Loading

I made a ton of improvements and am trying to settle on one of the three views for the color. I like the middle one the best (it is the current one in the code):

image

image

image

I changed the summary to be the database name and streamlined it into the main text on the unlock screen. If the name or color are not set then the unlock screen looks like it does before this change.

github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 2, 2024
View reviewed changes
src/gui/DatabaseOpenWidget.cpp
m_ui->displayColorLabel->setStyleSheet(
QString("background: %1; border: 1px solid palette(dark); border-radius: 4px").arg(color));

// m_ui->centralStack->setStyleSheet(QString("QStackedWidget {border: 4px solid %1}").arg(color));

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
@techninja1008
Copy link
Author

I quite like both the second and third ones, however I still quite like the original banner style and how in your face it was

@droidmonkey
Copy link
Member

droidmonkey commented Jun 2, 2024 •
edited
Loading

The original was ok, but looked like our warning/error banner to be honest. I wouldn't personally use this feature with the original look, but with the 2nd or 3rd I would. I also wanted to separate the database name from the color since some people would want one or the other. I'll let @phoerious weigh in before I make any more changes.

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

@phoerious phoerious Awaiting requested review from phoerious

@droidmonkey droidmonkey Awaiting requested review from droidmonkey

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
new feature user interface
Projects
None yet
Milestone
v2.8.0
Development

Successfully merging this pull request may close these issues.

Add description / colour label to lock screen
2 participants
@techninja1008 @droidmonkey