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

Fix window decorations #11440

Merged
merged 11 commits into from
Aug 2, 2024
Merged

Fix window decorations #11440

merged 11 commits into from
Aug 2, 2024

Conversation

calixtus
Copy link
Member

@calixtus calixtus commented Jun 30, 2024

Refs #10731

  • 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.

@calixtus calixtus added the ui label Jun 30, 2024
@calixtus calixtus marked this pull request as draft June 30, 2024 06:04
@calixtus calixtus added the status: waiting-for-feedback The submitter or other users need to provide more information about the issue label Jun 30, 2024
@calixtus calixtus changed the title Fix window decorations Fix window decorations (attempt) Jun 30, 2024
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

Reading https://ux.stackexchange.com/a/125593/93436 and https://unix.stackexchange.com/q/43106/18033, I think there should be an option whether JabRef should remember its size. Default should be yes on Windows and no on Linux.

I am even thinking to put "no" as default on Windows, because when users have "FancyZones" installed, the Window management should be better...

preferences.setPositionY(mainStage.getY());
preferences.setSizeX(mainStage.getWidth());
preferences.setSizeY(mainStage.getHeight());
}
preferences.setWindowMaximised(mainStage.isMaximized());
Copy link
Member

Choose a reason for hiding this comment

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

I would drop this line, too.

This should keep the fix at #4939

@koppor
Copy link
Member

koppor commented Jul 1, 2024

Note: The x values are "somehow" globally for multiple screens:

Example for

image

[javafx.stage.Screen@ede6e680 bounds:Rectangle2D [minX=0.0, minY=0.0, maxX=1600.0, maxY=2560.0, width=1600.0, height=2560.0] visualBounds:Rectangle2D [minX=0.0, minY=156.0, maxX=1600.0, maxY=2560.0, width=1600.0, height=2404.0] dpi:125.0 outputScale:(1.5,1.5), javafx.stage.Screen@42b22ab3 bounds:Rectangle2D [minX=5440.0, minY=96.0, maxX=6880.0, maxY=2656.0, width=1440.0, height=2560.0] visualBounds:Rectangle2D [minX=5440.0, minY=96.0, maxX=6880.0, maxY=2616.0, width=1440.0, height=2520.0] dpi:108.0 outputScale:(1.0,1.0), javafx.stage.Screen@21359912 bounds:Rectangle2D [minX=1600.0, minY=232.0, maxX=5440.0, maxY=2392.0, width=3840.0, height=2160.0] visualBounds:Rectangle2D [minX=1600.0, minY=232.0, maxX=5440.0, maxY=2352.0, width=3840.0, height=2120.0] dpi:139.0 outputScale:(1.0,1.0)]

@koppor
Copy link
Member

koppor commented Jul 1, 2024

I enabled debug output, added more things to debug output and included also more checks for "out of bounds". Lets see, if this matches more cases.

@koppor koppor changed the title Fix window decorations (attempt) Fix window decorations Jul 1, 2024
@koppor
Copy link
Member

koppor commented Jul 1, 2024

Note that the updated code also should keep #5037 fixed.

I did not add some new preference to store the JabRef X and Y position depending on the screen where JabRef "lives". This can be a follow up (if users ask for it). Update #7341 asks for it.

@koppor
Copy link
Member

koppor commented Jul 1, 2024

We could always save the screen position if user moves the window. In case the user fixes the window position and then maximizes JabRef and then quits, the user-fixed window position is not stored.

@koppor koppor mentioned this pull request Jul 1, 2024
6 tasks
@Siedlerchr
Copy link
Member

@calixtus calixtus marked this pull request as ready for review July 23, 2024 15:53
@calixtus calixtus added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers and removed status: waiting-for-feedback The submitter or other users need to provide more information about the issue labels Jul 23, 2024
@koppor koppor enabled auto-merge August 2, 2024 12:43
@koppor
Copy link
Member

koppor commented Aug 2, 2024

There was no follow-up on my PR comment. I think, it is good to go as is, since it brings things forward (and unblocks #11441).

Therefore, approved and added to the merge queue.

@koppor koppor added this pull request to the merge queue Aug 2, 2024
Copy link
Contributor

github-actions bot commented Aug 2, 2024

The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build.

Merged via the queue into main with commit 60282b4 Aug 2, 2024
24 of 25 checks passed
@koppor koppor deleted the fix-10731 branch August 2, 2024 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants