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

The text on the buttons is cut off when requesting a save #10381

Closed
xboxones1 opened this issue Mar 11, 2024 · 19 comments · Fixed by #10500
Closed

The text on the buttons is cut off when requesting a save #10381

xboxones1 opened this issue Mar 11, 2024 · 19 comments · Fixed by #10500

Comments

@xboxones1
Copy link
Contributor

xboxones1 commented Mar 11, 2024

Steps to Reproduce

  1. Disable autosave
  2. Make changes to the database
  3. Close keepassxc

KeePassXC - 2.7.7
Operating System: Windows/Linux

@xboxones1 xboxones1 added the bug label Mar 11, 2024
@droidmonkey
Copy link
Member

This is a bug with Qt on Linux. There is not much we can do to fix this problem

@xboxones1
Copy link
Contributor Author

The text in Windows is also cropped a bit around the edges of the buttons

@michaelk83
Copy link

KPXC uses Qt on all OSes. Try adjusting the Qt scaling.
https://keepassxc.org/docs/KeePassXC_UserGuide#_command_line_options

@xboxones1
Copy link
Contributor Author

@droidmonkey I've read up on this problem and as far as I understand it's not a bug. Need to resize the buttons. It's been looked at in more detail here

@droidmonkey
Copy link
Member

msgClose.layout()->setSizeConstraint(QLayout::SetMinimumSize);

This should be done by Qt, hence it is their bug. Sure we can do that ourselves (and we will, thank you for the share), but the UI framework should be handling overflow...

@droidmonkey droidmonkey added this to the v2.7.8 milestone Mar 17, 2024
@droidmonkey droidmonkey self-assigned this Mar 17, 2024
droidmonkey added a commit that referenced this issue Mar 18, 2024
@xboxones1
Copy link
Contributor Author

xboxones1 commented Mar 26, 2024

@droidmonkey Additionally, you need to change the base style as the size of dialogs and buttons is set there
https://github.com/keepassxreboot/keepassxc/blob/develop/src/gui/styles/base/basestyle.qss#L20-L22
I tested different values and came to the conclusion that this just needs to be removed, then the buttons scale correctly with the text.

@droidmonkey
Copy link
Member

That doesn't make sense, ugh, I hate Qt styling

@xboxones1
Copy link
Contributor Author

xboxones1 commented Mar 26, 2024

@droidmonkey without changing the base style, it doesn't work. Try building and see for yourself. This is also written about on stackoverflow, the link was above.

My final solution includes answer as well as a stylesheet padding setting.

And See UPDATE 3

@droidmonkey
Copy link
Member

droidmonkey commented Mar 26, 2024

This is still a qt bug...

droidmonkey added a commit that referenced this issue Mar 29, 2024
@xboxones1
Copy link
Contributor Author

xboxones1 commented Mar 30, 2024

@droidmonkey You also need to delete these three lines
https://github.com/keepassxreboot/keepassxc/blob/develop/src/gui/styles/base/basestyle.qss#L20-L22
This setting prevents buttons and dialog from being scaled. Additionally, if you know how, you can center the message.

This is after removing these lines in the style settings, scales correctly.
1

@droidmonkey
Copy link
Member

droidmonkey commented Mar 30, 2024

I will not remove those lines, we put that there for a reason, we want our dialogbox buttons to have a minimum width! Qt, however, treats that as license to squish the widget down to that minimum width without regard for the contents. That is a serious bug since there is no way to set a minimum width otherwise.

Qt even considers this a feature, which is insane to me:
image

I added an attempt to wipe out the min-width setting if the QDialogBox is within a QMessageBox widget, see here: https://github.com/keepassxreboot/keepassxc/pull/10500/files#diff-dc3ecb5ff0e9757970323ce929d8a40fcaef9b80421087bab00147186dbb4abd

Can you test this PR for me?

@xboxones1
Copy link
Contributor Author

xboxones1 commented Mar 30, 2024

After the fixes you've made, they will be minimum length anyway, and so you're just trimming the content of buttons and dialogs in other languages, the scaling won't work.

I've tested your PR and it doesn't work. That's why I'm writing about the minimum width restriction.

@droidmonkey
Copy link
Member

droidmonkey commented Mar 30, 2024

minimum length != minimum width. The min-width setting guarantees a button will be (in this case) at least 55px wide. So even if the text is "OK" it will be 55px wide. The widget should always be big enough to hold the contents unless some sort of overflow style is set, that is how CSS works.

Test it again if you haven't in the past day, I made a recent push.

@xboxones1
Copy link
Contributor Author

xboxones1 commented Mar 30, 2024

I saw your new PR that's exactly what I've been testing. I built it before I wrote it

@droidmonkey
Copy link
Member

I have another idea, just pushed, try it again

@xboxones1
Copy link
Contributor Author

xboxones1 commented Mar 30, 2024

I was wrong about the length, I meant the width of course.

It's not working.
1

@droidmonkey
Copy link
Member

OK, I replicated this on macOS, agree that doesn't work (I think cause the dialog is a child of MainWindow). Either way, I removed the min-width setting and nothing seems to be different in the application so I'll just remove it. Sorry about that, I should have tested just removing it to see if there were any poor effects in the first place.

@xboxones1
Copy link
Contributor Author

The text is left-aligned to the icon. Will you be able to center it?

@droidmonkey
Copy link
Member

Are you referring to the text within the message box? Why would it be centered, that wouldn't look good.

Also, we are programmatically enforcing an 80px min-width anyway:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants