Skip to content

Minor style fixes#11445

Merged
droidmonkey merged 4 commits intokeepassxreboot:developfrom
xboxones1:minor-style-fixes
Jan 2, 2025
Merged

Minor style fixes#11445
droidmonkey merged 4 commits intokeepassxreboot:developfrom
xboxones1:minor-style-fixes

Conversation

@xboxones1
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

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

@xboxones1
Copy link
Copy Markdown
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
Copy Markdown
Member

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

@xboxones1
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

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

@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
@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 <support@dmapps.us>
@droidmonkey droidmonkey added the pr: backported Pull request backported to previous release label Jan 19, 2025
pull bot pushed a commit to Graysonbarton/keepassxc that referenced this pull request Jan 26, 2025
* Clean up removed elements in qt stylesheets
* Disable main window when saving
* Fixed triangle size in group view

---------

Co-authored-by: Jonathan White <support@dmapps.us>
pull bot pushed a commit to blog2i2j/keepassxreboot.._..keepassxc that referenced this pull request Feb 22, 2025
* Clean up removed elements in qt stylesheets
* Disable main window when saving
* Fixed triangle size in group view

---------

Co-authored-by: Jonathan White <support@dmapps.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: backported Pull request backported to previous release theme

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants