Bugfix for Multiscreen #5738

MisterM13 · 2019-12-11T19:00:12Z

Made following Changes to solve Multiscreen issue fixes #5037:

At JabRefGUI:

new local variable correctedWindowPos to support saving of Screendata
change in method openWindow to open window at the mainscreen if needed
made method printWindowState() to print out windowposition data (if Logger in debug mode)
made method testExternalCoordinates() to test if window is on external screen
made method numberOfMonitors() to test if external screen is connected
( |-> numberOfMonitors() finally replaced by Screen.getScreens().size())

At JabRefPreferences:

made copy of the window position and size variables (recognised with _CORE)
made initialisation for the variables described above

(made in accordance with an Project on Software Engineering at University Basel in cooperation with @unibas-marcelluethi)

Change in CHANGELOG.md described (if applicable)
Manually tested changed features in running JabRef (always required)

…osition Variables in JabRefPreferences to support bugfix

Siedlerchr · 2019-12-11T19:11:37Z

Thanks for the contribution. Please remove the Travis integration as we recently switched to Github Actions.
Otherwise this looks good so far

tobiasdiez

Welcome to JabRef!

I've two suggestions to improve the code a little bit. Other than that, the code looks really good. It's also really helpful that you clearly outline your changes in the PR description. Thanks!

tobiasdiez · 2019-12-11T19:28:40Z

src/main/java/org/jabref/JabRefGUI.java

+     * returns the number of Monitors connected to the Computer
+     * @return numberOfmonitors
+     */
+    private int numberOfMonitors() {


I think the pure JavaFX version would be Screen.getScreens().size()

integrated with newest commit

tobiasdiez · 2019-12-11T19:29:38Z

src/main/java/org/jabref/JabRefGUI.java

+     */
+    private boolean testExternalCoordinates() {
+        boolean outbounds = false;
+        if (Globals.prefs.getDouble(JabRefPreferences.POS_X) > Globals.prefs.getDouble(JabRefPreferences.SIZE_X)) {


I think it would be slightly better to test against the actual dimensions of the screen, which you can get via Screen.getPrimary().getBounds()

integrated with newest commit

tobiasdiez

Thanks! The code looks good and I've only a few minor remarks before we can merge.

tobiasdiez · 2019-12-12T10:53:13Z

src/main/java/org/jabref/JabRefGUI.java

+    private void printWindowState(Stage mainStage) {
+        StringBuilder bob = new StringBuilder();
+        bob.append("SCREEN DATA:");
+        bob.append("JabRefPreferences.WINDOW_MAXIMISED: " + mainStage.isMaximized() + "\n");


Shouldn't JabRefPreferences be replaced by MainStage ? (As this does not print the values stored in the preferences)

yes it should, I think I had it wrong in because of earlier tries, were I also printed the preferences 😅 it'll be changed at the next commit

tobiasdiez · 2019-12-12T10:55:49Z

src/main/java/org/jabref/JabRefGUI.java

+     * Tests if the window coordinates are out of the mainscreen
+     * @return outbounds
+     */
+    private boolean testExternalCoordinates() {


test is a bit vague, what about storedWindowPositionIsOutOfBounds (or maybe something a bit shorter).

renamed it with isWindowPositionOutOfBounds()

tobiasdiez · 2019-12-12T10:58:44Z

src/main/java/org/jabref/preferences/JabRefPreferences.java

@@ -172,6 +172,10 @@
    public static final String SIZE_X = "mainWindowSizeX";
    public static final String POS_Y = "mainWindowPosY";
    public static final String POS_X = "mainWindowPosX";
+    public static final String SIZE_Y_CORE = "mainWindowSizeYCore";


As the user can never configure these values, there is no need to add them as preferences. Please convert them to static fields in JabRefGUI.

I will try to set the Numbers right in at the method openWindow(), not sure if that works, but if it do I can delete all variables I made up..

…med method testExternalCoordinates with isWindowPositionOutOfBounds(), Deleted unused Variables at JabrefPreferences due to a correction in openWindow (at JabRefGUI)

tobiasdiez · 2019-12-12T20:16:26Z

Thanks for the quick follow-up!

* upstream/master: (21 commits) New Crowdin translations (#5729) Update license of Oracle JDBC Changes string representation when detecting custom entry types. (#5741) Try to fix GitVersion Also run workflow on tag creation Add README.md with an initial help to work on command line para… (#5720) Bugfix for Multiscreen (#5738) Fix #2868 - keep source groups selected after drag and drop retrigger build Squashed 'src/main/resources/csl-styles/' changes from f71cd32..49a1841 try to fix pull of csl styles try to fix csl update Fixed untranslated Strings issue #5701 Fix #5603 - account for file extension in file name length (#5726) New translations JabRef_en.properties (French) Bump unirest-java from 3.2.00 to 3.3.00 Bump postgresql from 42.2.8 to 42.2.9 Bump tika-core from 1.22 to 1.23 Fix upload to GitHub artifacts (#5712) Try to fix csl update (#5718) ...

60bf7d5 Add encyclopedia type to wikipedia-templates.csl (#5778) 031afe1 Update and rename dependent/organization-studies.csl to organization-… (#5779) 7ed71e7 Update harvard-newcastle-university.csl (#5765) 46bab91 Update annals-of-oncology.csl (#5760) 6158ae6 Create research-in-plant-disease.csl (#5738) 04422a8 Create chemmedchem.csl (#5753) 7c11521 Create clinical-kidney-journal.csl (#5749) e7ee983 Create kit-karlsruher-institut-fur-technologie-germanistik-ndl-neuere… (#5729) 96a1106 Update article format for STM journal (#5755) a4ca057 Update historia-scribere.csl (#5748) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: 60bf7d5

MisterM13 added 4 commits December 11, 2019 15:20

Travis integration

b02ce7b

Bugfix for issue 5037: made several methods on JabRefGUI and second p…

7b2544a

…osition Variables in JabRefPreferences to support bugfix

made comments and corrected Checkstyle

8f5cd01

added my change/bugfix to Changelog

6792010

tobiasdiez requested changes Dec 11, 2019

View reviewed changes

MisterM13 added 2 commits December 11, 2019 21:35

removed .travis.yml

31a2c1a

made suggested Changes from tobiasdiez

031dbc5

MisterM13 marked this pull request as ready for review December 11, 2019 21:39

Siedlerchr approved these changes Dec 12, 2019

View reviewed changes

tobiasdiez requested changes Dec 12, 2019

View reviewed changes

MisterM13 and others added 2 commits December 12, 2019 14:03

replaced JabrefPreferences with mainStage at printWindowState(), rena…

e652b38

…med method testExternalCoordinates with isWindowPositionOutOfBounds(), Deleted unused Variables at JabrefPreferences due to a correction in openWindow (at JabRefGUI)

Merge branch 'master' into Bugfix

b26e243

tobiasdiez merged commit 0aa6138 into JabRef:master Dec 12, 2019

koppor mentioned this pull request Dec 20, 2019

Fix code style #5772

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bugfix for Multiscreen #5738

Bugfix for Multiscreen #5738

MisterM13 commented Dec 11, 2019 •

edited

Loading

Siedlerchr commented Dec 11, 2019

tobiasdiez left a comment

tobiasdiez Dec 11, 2019

MisterM13 Dec 11, 2019

tobiasdiez Dec 11, 2019

MisterM13 Dec 11, 2019

tobiasdiez left a comment

tobiasdiez Dec 12, 2019

MisterM13 Dec 12, 2019

tobiasdiez Dec 12, 2019

MisterM13 Dec 12, 2019

tobiasdiez Dec 12, 2019

MisterM13 Dec 12, 2019

tobiasdiez commented Dec 12, 2019

Bugfix for Multiscreen #5738

Bugfix for Multiscreen #5738

Conversation

MisterM13 commented Dec 11, 2019 • edited Loading

Siedlerchr commented Dec 11, 2019

tobiasdiez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasdiez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasdiez commented Dec 12, 2019

MisterM13 commented Dec 11, 2019 •

edited

Loading