Enhance side pane toggle #1605

Braunch · 2016-07-19T18:02:29Z

I improved the SidePanes toggle functionality. As described by @simonharrer at #1225 (comment) the components should be shown, get focused if shown but not focused and closed when shown and focused.
Done for:

GroupSelector
GeneralFetcher
OpenOfficePanel

Bonus:

EntryEditor/PreviewPanel

Update:
The panes extending the SidePaneComponent have implemented toggle functionality. To finish the task i depend on @mairdl 's PR: #1525 and his work on the group functionality. To make the toggle visible on the groupsSelector, the tree needs to be focusable and @mairdl is implementing it. Also the OOPanel depends on having a hotkey since the three phase toggling does not make sense when toggling using the mouse (you could click the panel right away instead of the menu item). So this PR will be on hold (hopefully) just for some days.

Braunch · 2016-07-19T18:07:28Z

@simonharrer how should the toggle for the preview panel and the entry editor work in your opinion?
Since the preview has nothing to focus the three phase toggle is not suitable here. For the entry editor on the other hand, I can imagine using the three phases. Any opinions on that (from anyone of course, just refering to @simonharrer because it was him to mention the idea in the first place)?

koppor · 2016-09-12T21:03:57Z

#1525 is merged. Will there be updates on this PR?

chriba · 2016-09-27T10:44:58Z

I took this issue to myself and coded a quick solution.

OpenOffice/LibreOffice is now able to be toggled.

tschechlovdev

Some minor comments, but in general LGTM

tschechlovdev · 2016-09-28T14:05:20Z

src/main/java/net/sf/jabref/gui/SidePaneManager.java

@@ -67,6 +66,11 @@ public synchronized boolean isComponentVisible(String name) {
        }
    }

+    /**
+     * if panel is visible it will be hidden and the other way around


Please start comments with Capital letters.

tschechlovdev · 2016-09-28T14:05:35Z

src/main/java/net/sf/jabref/gui/SidePaneManager.java

@@ -75,6 +79,21 @@ public synchronized void toggle(String name) {
        }
    }

+    /**
+     * if panel is hidden it will be shown and focused


Same as above.

tschechlovdev · 2016-09-28T14:05:53Z

src/main/java/net/sf/jabref/gui/SidePaneManager.java

+    /**
+     * if panel is hidden it will be shown and focused
+     * if panel is visible but not focused it will be focused
+     * ig panel is visible and focused it will be hidden


Typo ig -> if

tschechlovdev · 2016-09-28T14:07:03Z

src/main/java/net/sf/jabref/gui/SidePaneManager.java

+     * @param name name of the panel
+     */
+    public synchronized void toggleThreeWay(String name){
+        if (isComponentVisible(name) && Globals.getFocusListener().getFocused() == components.get(name)) {


Would be nice if you could add additional brackets here, that would make it clearer.

I extracted the second condition

tschechlovdev · 2016-09-28T14:10:48Z

src/main/java/net/sf/jabref/gui/groups/GroupSelector.java

@@ -702,6 +712,11 @@ public void componentOpening() {
    }

    @Override
+    public String getSidePaneName() {


Hm why do you set NAME as public static final and add this get method? Isn't this unnecessary then?

I need to access the name from the SidePaneComponent.
Sadly you cannot define an abstract static method

koppor · 2016-10-10T21:35:59Z

Refs koppor#54

# Conflicts: # src/test/java/net/sf/jabref/gui/importer/fetcher/GeneralFetcherTest.java

koppor · 2016-10-11T02:53:52Z

I tried it: Works good. Code LGTM.

tobiasdiez

Just some minor remarks/questions from my side. In general LGTM.

tobiasdiez · 2016-10-11T08:52:16Z

src/main/java/net/sf/jabref/collab/FileUpdatePanel.java

@@ -69,6 +68,11 @@ public void componentClosing() {
    }

    @Override
+    public String getSidePaneName() {
+        return NAME;


Can one simply return "fileUpdate" here and replace all references to NAME with calls to getSidePaneName?
Applies also to the other side panes.
One probably needs to recode getComponent to not use the name but the class (as a generic version <T> T getComponent(Class<T> clazz))

I used the static NAME to change the state of the side panels without having an object of it at hand.
I changed the SidePaneManager to manage the side panels in a generic way.

tobiasdiez · 2016-10-11T09:08:16Z

src/main/java/net/sf/jabref/gui/JabRefFrame.java

@@ -1519,8 +1504,8 @@ private void initActions() {
        openDatabaseOnlyActions.addAll(Arrays.asList(mergeDatabaseAction, newSubDatabaseAction, save, globalSearch,
                saveAs, saveSelectedAs, saveSelectedAsPlain, editModeAction, undo, redo, cut, deleteEntry, copy, paste, mark, markSpecific, unmark,
                unmarkAll, rankSubMenu, editEntry, selectAll, copyKey, copyCiteKey, copyKeyAndTitle, copyKeyAndLink, editPreamble, editStrings,
-                toggleGroups, makeKeyAction, normalSearch, generalFetcher.getAction(), mergeEntries, cleanupEntries, exportToClipboard, replaceAll,
-                sendAsEmail, downloadFullText, writeXmpAction, optMenuItem, findUnlinkedFiles, addToGroup, removeFromGroup,
+                groupSelector.getAction(), makeKeyAction, normalSearch, generalFetcher.getAction(), mergeEntries, cleanupEntries, exportToClipboard, replaceAll,


Maybe rename getAction -> getToggleAction ?

tobiasdiez · 2016-10-11T09:09:35Z

src/main/java/net/sf/jabref/gui/groups/GroupSelector.java

+        groupsTree.grabFocus();
+    }
+
+    public ToggleAction getAction() {


Make getAction an abstract method in SidePaneComponent ?

Done, although FileUpdatePanel doesn't implements it.

tobiasdiez · 2016-10-11T09:11:33Z

src/main/java/net/sf/jabref/gui/openoffice/OpenOfficePanel.java

-    public String getName() {
-        return "OpenOffice/LibreOffice";
+    public SidePaneComponent.ToggleAction getAction() {
+        return sidePane.getToggleAction();
    }


    private class OOPanel extends SidePaneComponent {


Can you please extract this private class to a new file. Thanks.

chriba · 2016-10-11T17:41:37Z

~~When a subclass does NOT implement an abstract method, how should I mark it?~~
Nevermind, the UnsupportedOperationException seems a good option.

tobiasdiez · 2016-10-12T11:35:38Z

Code looks good to me now (thanks for the quick changes!). I'll merge it in.

* upstream/master: (24 commits) hotfix NPE in the main table (#2158) Enhance side pane toggle (#1605) Added gray background text to authors field to assist newcomers (#2147) Rewrite google scholar fetcher to new infrastructure (#2101) Found entries will be shown when dropping a pdf with xmp meta data (#2150) Japanese translation (#2155) Add shortcuts to context menu (#2131) [WIP] Doi resolution ignore (#2154) Fix DuplicationChecker and key generator (#2151) just close popup on first ESC keep entry in sight Fix isSharedDatabaseAlreadyPresent check. don't change entry after search if editing an entry (#2129) Change download URL to downloads.jabref.org (#2145) fix switching edited textfield in the entry editor with TAB (#2138) Update me.champeau.gradle.jmh' version from 0.3.0 to 0.3.1 Update install4j plugin from 6.1.2 to 6.1.3 Remove gradle plugin com.github.youribonnaffe.gradle.format as it is deprecated and the config uses a non-existing file Update gradle from 3.0 to 3.1 Fix changing the font size not working when importing preferences (#2069) ... # Conflicts: # src/main/java/net/sf/jabref/gui/externalfiles/DroppedFileHandler.java # src/main/java/net/sf/jabref/gui/importer/fetcher/GoogleScholarFetcher.java

Braunch added type: enhancement ui stupro labels Jul 19, 2016

Braunch self-assigned this Jul 19, 2016

Braunch force-pushed the enhanceSidePaneToggle branch 3 times, most recently from 3ed9ae8 to 8509239 Compare July 26, 2016 13:47

Braunch mentioned this pull request Aug 8, 2016

Revised hotkey system #1525

Merged

chriba force-pushed the enhanceSidePaneToggle branch 2 times, most recently from 960467f to b3e48c4 Compare September 27, 2016 10:43

chriba force-pushed the enhanceSidePaneToggle branch 3 times, most recently from 5532df6 to 99da73c Compare September 27, 2016 10:54

boceckts added the stupro-ready-for-internal-review label Sep 27, 2016

chriba force-pushed the enhanceSidePaneToggle branch from 99da73c to 482db82 Compare September 27, 2016 14:34

boceckts removed the stupro-ready-for-internal-review label Sep 28, 2016

chriba force-pushed the enhanceSidePaneToggle branch 5 times, most recently from 64781e9 to 6d60cd6 Compare September 28, 2016 13:14

enhance toggle behaviour

0de22f1

chriba force-pushed the enhanceSidePaneToggle branch from 6d60cd6 to 0de22f1 Compare September 28, 2016 13:16

tschechlovdev reviewed Sep 28, 2016

View reviewed changes

tschechlovdev assigned chriba Sep 28, 2016

fix issues

8f454e7

tschechlovdev added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Sep 28, 2016

koppor changed the title ~~[WIP] Enhance side pane toggle~~ Enhance side pane toggle Sep 28, 2016

chriba force-pushed the enhanceSidePaneToggle branch from cbcc4de to 8f454e7 Compare October 10, 2016 21:48

Merge remote-tracking branch 'jabref/master' into enhanceSidePaneToggle

9e07cee

# Conflicts: # src/test/java/net/sf/jabref/gui/importer/fetcher/GeneralFetcherTest.java

tobiasdiez requested changes Oct 11, 2016

View reviewed changes

chriba added 3 commits October 11, 2016 11:26

make getToggleAction abstract

2317d85

make SideComponentManager generic

7c5179a

extract OOPanel

b339cd1

koppor removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Oct 11, 2016

fix compile error

00f255b

tobiasdiez approved these changes Oct 11, 2016

View reviewed changes

tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Oct 11, 2016

change method not implemented exception

f6564a8

tobiasdiez merged commit 56b9a67 into JabRef:master Oct 12, 2016

chriba deleted the enhanceSidePaneToggle branch October 12, 2016 11:36

Braunch mentioned this pull request Oct 17, 2016

Hotkey-system should be consistent #1225

Closed

koppor mentioned this pull request May 23, 2023

Fix issue 9863 - Change Tab selection order #9907

Merged

koppor added the keybinding label May 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance side pane toggle #1605

Enhance side pane toggle #1605

Braunch commented Jul 19, 2016 •

edited

Loading

Braunch commented Jul 19, 2016

koppor commented Sep 12, 2016

chriba commented Sep 27, 2016 •

edited

Loading

tschechlovdev left a comment

tschechlovdev Sep 28, 2016

chriba Sep 28, 2016

tschechlovdev Sep 28, 2016

chriba Sep 28, 2016

tschechlovdev Sep 28, 2016

chriba Sep 28, 2016

tschechlovdev Sep 28, 2016

chriba Sep 28, 2016

tschechlovdev Sep 28, 2016

chriba Sep 28, 2016

koppor commented Oct 10, 2016

koppor commented Oct 11, 2016

tobiasdiez left a comment

tobiasdiez Oct 11, 2016 •

edited

Loading

chriba Oct 11, 2016

tobiasdiez Oct 11, 2016

chriba Oct 11, 2016

tobiasdiez Oct 11, 2016

chriba Oct 11, 2016

tobiasdiez Oct 11, 2016

chriba Oct 11, 2016

chriba commented Oct 11, 2016 •

edited

Loading

tobiasdiez commented Oct 12, 2016

Enhance side pane toggle #1605

Enhance side pane toggle #1605

Conversation

Braunch commented Jul 19, 2016 • edited Loading

Braunch commented Jul 19, 2016

koppor commented Sep 12, 2016

chriba commented Sep 27, 2016 • edited Loading

tschechlovdev 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

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

koppor commented Oct 10, 2016

koppor commented Oct 11, 2016

tobiasdiez left a comment

Choose a reason for hiding this comment

tobiasdiez Oct 11, 2016 • edited Loading

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

Choose a reason for hiding this comment

chriba commented Oct 11, 2016 • edited Loading

tobiasdiez commented Oct 12, 2016

Braunch commented Jul 19, 2016 •

edited

Loading

chriba commented Sep 27, 2016 •

edited

Loading

tobiasdiez Oct 11, 2016 •

edited

Loading

chriba commented Oct 11, 2016 •

edited

Loading