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

Maximize the duplicate entries dialog #9278

Closed
wants to merge 1 commit into from

Conversation

u7306160
Copy link

@u7306160 u7306160 commented Oct 22, 2022

Fixes #9055

I think this issue is asking to make the merge dialog as large as possible but within the size of the screen. Initially, when the main window of Jabref is placed in the top left-hand corner of the screen, the merge pop-up is off-screen. My solution is to get
the primary screen bound first, then set the initial size of the dialog based on the primary screen bound. So after the first run of the new dialog, it will show all the merge information with no part of it out-of-screen, although it's not a full-screen show. It has the maximum width, but not the highest height.

I kept it able to retrieve the previous window state and set the new dialog window size and position to match it. Since it's initially set to maximum, running it again without changing its size will keep the dialog large.

dialog

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Comment on lines +60 to +64
Rectangle2D primaryScreenBounds = Screen.getPrimary().getVisualBounds();
this.setX(primaryScreenBounds.getMinX());
this.setY(primaryScreenBounds.getMinY());
this.setWidth(primaryScreenBounds.getWidth());
this.setHeight(primaryScreenBounds.getHeight());
Copy link
Member

Choose a reason for hiding this comment

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

This always maximizes the dialog. IMHO, the dialog position should be changed if the new position would be not completely in the screen / completely visible. Other than that, the "old" size should be used. I am not sure how to implement that - maybe let this handled by the operating system and no additional code by JabRef.

@ThiloteE
Copy link
Member

ThiloteE commented Oct 27, 2022

Today I had a look at this pr and tested it with Linux Mint Cinnamon (1366x768) and a second screen (1920x1080) and it all worked perfectly (except that my mouse sometimes was hitting a slight "barrier" when moving from screen 1 to screen 2 and vice versa, but I think that is probably not caused by JabRef). It fully fixes issue #9055. Edit: turns out, I do not experience the initial issue as described in issue 9055 at all on my Linux Mint Cinnamon machine, so I cannot test and make a judgement about the state of this pr.

Great job! Seems ready to merge, unless there are issues with quality of the code.

Edit: I have such a small screen on my laptop, that the original size is almost as big as a maximized screen, so I barely notice a difference, but I could imagine that people with a larger screen size would not want this window to maximize everytime it pops up...

@koppor
Copy link
Member

koppor commented Oct 27, 2022

Edit: I have such a small screen on my laptop, that the original size is almost as big as a maximized screen, so I barely notice a difference, but I could imagine that people with a larger screen size would not want this window to maximize everytime it pops up...

This was the point of my code comment! - I have a screen resolution of 3440x1440 and I don't want that an application automatically maximizes a window. Especially at the merge entries dialog, I cannot really get what belongs togehter:

grafik

@calixtus
Copy link
Member

calixtus commented Nov 8, 2022

What's the status here?

@calixtus calixtus added the status: waiting-for-feedback The submitter or other users need to provide more information about the issue label Nov 8, 2022
@ThiloteE
Copy link
Member

The status is following: The PR works, but Koppor does not like maximized screen everytime there is a dialogue popup... so I am closing to reduce the number of open pull-requests

@ThiloteE ThiloteE closed this Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-feedback The submitter or other users need to provide more information about the issue ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merge Entries Dialog should render properly at smaller screen resolutions
4 participants