Enhance side pane toggle by Braunch · Pull Request #1605 · JabRef/jabref

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

    }

+    /**
+     * 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

    }

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


Same as above.

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

+    /**
+     * 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

+     * @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

        valueChanged(null);
    }

+    @Override


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 JabRef#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

    }

+    @Override
+    public String getSidePaneName() {


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

                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

+        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

    }


    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 [outdated] type: enhancement component: 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 component: keybinding label May 23, 2023

Uh oh!

Conversation

Braunch commented Jul 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Braunch commented Jul 19, 2016

Uh oh!

koppor commented Sep 12, 2016

Uh oh!

chriba commented Sep 27, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tschechlovdev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

koppor commented Oct 10, 2016

Uh oh!

koppor commented Oct 11, 2016

Uh oh!

tobiasdiez left a comment

Choose a reason for hiding this comment

Uh oh!

tobiasdiez Oct 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chriba commented Oct 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tobiasdiez commented Oct 12, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

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