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.

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

Minor style fixes #11445

Merged
merged 4 commits into from
Jan 2, 2025
Merged

Conversation

xboxones1
Copy link
Contributor

@xboxones1 xboxones1 commented Nov 7, 2024

  • Clean up removed elements
  • Disable the main window while saving to match the style
  • Fixed triangle size when entries are expanded

Screenshot

Main Window

Before
save-before
save-yubikey-before
After
save-after
save-yubikey-after

Triangle

1 2

1 2

Testing strategy

Manually

Type of change

  • ✅ Bug fix (non-breaking change that fixes an issue)

@droidmonkey
Copy link
Member

I like the After on your initial post the best, so your proposed change should be applied.

@droidmonkey droidmonkey added this to the v2.7.10 milestone Nov 8, 2024
@xboxones1 xboxones1 marked this pull request as draft December 4, 2024 06:53
@droidmonkey
Copy link
Member

droidmonkey commented Dec 7, 2024

In DatabaseWidget::performSave() we should be disabling the preview widget as well. It might just be more effective to disable the entire DatabaseWidget itself (ie, just call setEnabled(false)) instead of disable individual sub-widgets.

We don't disable the main window because you can switch tabs during save operations without impact to saving. Technically we can do the same when issuing the yubikey notice message, but we didn't.

@xboxones1
Copy link
Contributor Author

Saving happens literally in milliseconds even on weak arm devices, I don't think you can somehow manage to do something in that time. If the database is protected by yubikey, a popup notification appears first, disabling all elements, and only then the save takes place. Right now it is impossible to achieve uniformity of colors in different operations. With my edits I fixed the colors only when saving, removing from the styles the elements that did not match the color when they are disabled, but when the pop-up notification from yubikey appears, the colors do not match again, since all elements are disabled here. The easiest thing to do without editing the base style is to just disable everything during save operations. Or disable the same elements when the yubikey popup window appears, but here you will have to change the base style as well.

@droidmonkey
Copy link
Member

I'm not sure why we disable the main window when waiting for yubikey press

@xboxones1
Copy link
Contributor Author

xboxones1 commented Dec 8, 2024

I tested with disabling the screen when saving to show how it would look. It turned out just perfect, without any additional edits to the base style.

DatabaseWidget::performSave()


QWidget *mainWindow = this->window();
if (mainWindow) {
    mainWindow->setDisabled(true);
}
QApplication::processEvents();

if (mainWindow) {
    mainWindow->setDisabled(false); 
}

test

@droidmonkey
Copy link
Member

I said to not disable the whole main window, just the database widget itself.

@xboxones1
Copy link
Contributor Author

I see what you're suggesting, that you disable DatabaseWidget, but that won't change anything, the colors won't match either. I suggested what I think would be better to do with minimal code changes is to disable the window. I don't understand why you are against it? The database and its changes are saved instantly. You won't have time to switch to any tab. I don't understand what practical sense it makes to leave any element enabled when saving?

@phoerious
Copy link
Member

phoerious commented Dec 11, 2024

We disable the window during the key transformation and save operation, because there is no safe way we could modify the database at that time. I believe external reload triggers are also disabled for the duration. We had a few bugs in the past when users messed with their database while it was being saved. Hence we prevent modifications during saves and disable the UI to reflect that. The YubiKey button press occurs somewhere in the middle of that.

@droidmonkey
Copy link
Member

Thank you, I will check it out when I get a moment

@xboxones1 xboxones1 force-pushed the minor-style-fixes branch 3 times, most recently from c8dd35c to c8072b1 Compare December 18, 2024 01:08
@xboxones1 xboxones1 marked this pull request as ready for review December 18, 2024 03:16
@droidmonkey droidmonkey merged commit 2afec91 into keepassxreboot:develop Jan 2, 2025
11 checks passed
@xboxones1 xboxones1 deleted the minor-style-fixes branch January 2, 2025 12:27
droidmonkey added a commit that referenced this pull request Jan 3, 2025
* Clean up removed elements in qt stylesheets
* Disable main window when saving
* Fixed triangle size in group view

---------

Co-authored-by: Jonathan White <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants