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

lower minimum window height for ResolveConflictsDialog #7791

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nilsding
Copy link
Member

fixes #7785

IMO there's no need for the ResolveConflictsDialog to be that tall, especially when it's just one or two files conflicting... and there's a scrollview anyway.

fwiw the QWidgets-based OCC::ConflictDialog that's still used to resolve only single file-conflicts has a height of 407 defined in its .ui file

Screenshot_20250124_111408

@nilsding nilsding added this to the 3.16.0 milestone Jan 24, 2025
@nilsding nilsding self-assigned this Jan 24, 2025
Copy link
Collaborator

@claucambra claucambra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a sensible change but I think there may be even more improvements we can make here to reduce the vertical space -- the button box at the bottom seems t have excessive padding for example. Food for thought :)

@alexknop
Copy link

My apologies, I was was working on this in parallel in #7815

This also appears to fix #6228

It is an annoying issue for some screens like mine.
My only complaint is the height when I opened my dialog looked like this
Screenshot From 2025-01-31 12-55-11

I was able to drag down the dialog box to a more appropriate height.

Perhaps we can combine our changes:

  1. Lowering the minimum height to 500 in Style.qml
  2. Making Height equal to Math.min(contentItem.height + 20, Screen.height * 0.8) in ResolveConflictsDialog.qml

@nilsding nilsding linked an issue Feb 6, 2025 that may be closed by this pull request
8 tasks
@mgallien mgallien force-pushed the bugfix/change-resolve-conflicts-dialog-height branch from 1b14546 to d0c3b29 Compare February 7, 2025 14:49
@mgallien mgallien enabled auto-merge February 7, 2025 14:49
Copy link

github-actions bot commented Feb 7, 2025

Artifact containing the AppImage: nextcloud-appimage-pr-7791.zip

SHA256 checksum: 9afde478fa3c54fe0243a549158be2b4edcf98da08c4921b018dd548a0604051

To test this change/fix you can download the above artifact file, unzip it, and run it.

Please make sure to quit your existing Nextcloud app and backup your data.

Copy link

sonarqubecloud bot commented Feb 7, 2025

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