-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Add button-icon for union/intersection in the groups side panel #3954
Changes from 6 commits
8b9e6a9
1a72f7d
04ed6b3
971f384
4e3757b
c2ec176
099d579
ae1a438
9244e85
17a4c77
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,11 @@ | ||
package org.jabref.gui; | ||
|
||
import java.util.Optional; | ||
|
||
import javafx.scene.Node; | ||
import javafx.scene.control.Button; | ||
import javafx.scene.control.Label; | ||
import javafx.scene.control.Tooltip; | ||
import javafx.scene.layout.BorderPane; | ||
import javafx.scene.layout.HBox; | ||
import javafx.scene.layout.Priority; | ||
|
@@ -11,20 +14,23 @@ | |
import org.jabref.gui.actions.SimpleCommand; | ||
import org.jabref.gui.icon.IconTheme; | ||
import org.jabref.gui.icon.JabRefIcon; | ||
import org.jabref.logic.l10n.Localization; | ||
|
||
public abstract class SidePaneComponent { | ||
|
||
protected final SidePaneManager manager; | ||
protected final ToggleCommand toggleCommand; | ||
private final JabRefIcon icon; | ||
private final String title; | ||
protected final JabRefIcon icon; | ||
protected final String title; | ||
private Node contentNode; | ||
|
||
|
||
public SidePaneComponent(SidePaneManager manager, JabRefIcon icon, String title) { | ||
this.manager = manager; | ||
this.icon = icon; | ||
this.title = title; | ||
this.toggleCommand = new ToggleCommand(this); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove |
||
} | ||
|
||
protected void hide() { | ||
|
@@ -35,11 +41,11 @@ protected void show() { | |
manager.show(this.getType()); | ||
} | ||
|
||
private void moveUp() { | ||
protected void moveUp() { | ||
manager.moveUp(this); | ||
} | ||
|
||
private void moveDown() { | ||
protected void moveDown() { | ||
manager.moveDown(this); | ||
} | ||
|
||
|
@@ -90,28 +96,38 @@ public final Node getContentPane() { | |
*/ | ||
public final Node getHeader() { | ||
Button close = IconTheme.JabRefIcons.CLOSE.asButton(); | ||
close.setTooltip(new Tooltip(Localization.lang("Hide panel"))); | ||
close.setOnAction(event -> hide()); | ||
|
||
Button up = IconTheme.JabRefIcons.UP.asButton(); | ||
up.setTooltip(new Tooltip(Localization.lang("Move panel up"))); | ||
up.setOnAction(event -> moveUp()); | ||
|
||
Button down = IconTheme.JabRefIcons.DOWN.asButton(); | ||
down.setTooltip(new Tooltip(Localization.lang("Move panel down"))); | ||
down.setOnAction(event -> moveDown()); | ||
|
||
HBox buttonContainer = new HBox(); | ||
buttonContainer.getChildren().addAll(up, down, close); | ||
final HBox buttonContainer = new HBox(); | ||
buttonContainer.getChildren().addAll(up, down); | ||
getAddtionalHeaderButtons().ifPresent(btn -> buttonContainer.getChildren().add(btn)); | ||
buttonContainer.getChildren().add(close); | ||
|
||
BorderPane graphic = new BorderPane(); | ||
graphic.setCenter(icon.getGraphicNode()); | ||
BorderPane container = new BorderPane(); | ||
// container.setLeft(graphic); | ||
|
||
final Label label = new Label(title); | ||
BorderPane container = new BorderPane(); | ||
container.setCenter(label); | ||
container.setRight(buttonContainer); | ||
container.getStyleClass().add("sidePaneComponentHeader"); | ||
|
||
return container; | ||
} | ||
|
||
protected Optional<Node> getAddtionalHeaderButtons() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just from the name of the method, I would have expected a list of buttons as the return value and not a single one. I think, supporting multiple buttons make sense and is not a big change overall. |
||
return Optional.empty(); | ||
} | ||
|
||
/** | ||
* Create the content of this component | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,13 @@ | ||
package org.jabref.gui.groups; | ||
|
||
import java.util.Optional; | ||
|
||
import javafx.scene.Node; | ||
import javafx.scene.control.Button; | ||
import javafx.scene.control.Tooltip; | ||
import javafx.scene.layout.Priority; | ||
|
||
import org.jabref.gui.DialogService; | ||
import org.jabref.gui.SidePaneComponent; | ||
import org.jabref.gui.SidePaneManager; | ||
import org.jabref.gui.SidePaneType; | ||
|
@@ -20,15 +25,43 @@ | |
public class GroupSidePane extends SidePaneComponent { | ||
|
||
private final JabRefPreferences preferences; | ||
private final DialogService dialogService; | ||
private final Button intersectionUnionToggle = IconTheme.JabRefIcons.WWW.asButton(); | ||
private final Tooltip toggleUnion = new Tooltip(Localization.lang("Toogle union")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there is no need to have these tooltips as instance variables. I would simply inline them. |
||
private final Tooltip toggleIntersectoin = new Tooltip(Localization.lang("Toogle intersection")); | ||
|
||
public GroupSidePane(SidePaneManager manager, JabRefPreferences preferences) { | ||
public GroupSidePane(SidePaneManager manager, JabRefPreferences preferences, DialogService dialogService) { | ||
super(manager, IconTheme.JabRefIcons.TOGGLE_GROUPS, Localization.lang("Groups")); | ||
this.preferences = preferences; | ||
this.dialogService = dialogService; | ||
} | ||
|
||
@Override | ||
protected Optional<Node> getAddtionalHeaderButtons() { | ||
intersectionUnionToggle.setOnAction(event -> toggleUnionIntersection()); | ||
return Optional.of(intersectionUnionToggle); | ||
} | ||
|
||
private Node getUnionIntersectionGraphic() { | ||
return preferences.getGroupViewMode().getIcon().getGraphicNode(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally, I would extract these methods to a new class |
||
} | ||
|
||
private Tooltip getUnionInterSectionToolTip() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Intersection" and "Tooltip" |
||
GroupViewMode mode = preferences.getGroupViewMode(); | ||
if (mode == GroupViewMode.UNION) { | ||
return toggleIntersectoin; | ||
} | ||
if (mode == GroupViewMode.INTERSECTION) { | ||
return toggleUnion; | ||
} | ||
return new Tooltip(); | ||
} | ||
|
||
@Override | ||
public void afterOpening() { | ||
preferences.putBoolean(JabRefPreferences.GROUP_SIDEPANE_VISIBLE, Boolean.TRUE); | ||
intersectionUnionToggle.setGraphic(getUnionIntersectionGraphic()); | ||
intersectionUnionToggle.setTooltip(getUnionInterSectionToolTip()); | ||
} | ||
|
||
@Override | ||
|
@@ -46,6 +79,24 @@ public Action getToggleAction() { | |
return StandardActions.TOGGLE_GROUPS; | ||
} | ||
|
||
private void toggleUnionIntersection() { | ||
GroupViewMode mode = preferences.getGroupViewMode(); | ||
|
||
if (mode == GroupViewMode.UNION) { | ||
preferences.setGroupViewMode(GroupViewMode.INTERSECTION); | ||
dialogService.notify(Localization.lang("Group view mode set to intersection")); | ||
} | ||
|
||
if (mode == GroupViewMode.INTERSECTION) { | ||
preferences.setGroupViewMode(GroupViewMode.UNION); | ||
|
||
dialogService.notify(Localization.lang("Group view mode set to union")); | ||
} | ||
|
||
intersectionUnionToggle.setGraphic(getUnionIntersectionGraphic()); | ||
intersectionUnionToggle.setTooltip(getUnionInterSectionToolTip()); | ||
} | ||
|
||
@Override | ||
protected Node createContentPane() { | ||
return ViewLoader.view(GroupTreeView.class) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
package org.jabref.gui.groups; | ||
|
||
import org.jabref.gui.icon.IconTheme.JabRefIcons; | ||
|
||
public enum GroupViewMode { | ||
|
||
INTERSECTION(JabRefIcons.GROUP_INTERSECTION), | ||
UNION(JabRefIcons.GROUP_UNION); | ||
|
||
private JabRefIcons icon; | ||
|
||
GroupViewMode(JabRefIcons icon) { | ||
this.icon = icon; | ||
} | ||
|
||
GroupViewMode() { | ||
//empty, but needed for valueOf Method | ||
} | ||
|
||
public JabRefIcons getIcon() { | ||
return icon; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,6 +43,7 @@ | |
import org.jabref.gui.autocompleter.AutoCompletePreferences; | ||
import org.jabref.gui.desktop.JabRefDesktop; | ||
import org.jabref.gui.entryeditor.EntryEditorTabList; | ||
import org.jabref.gui.groups.GroupViewMode; | ||
import org.jabref.gui.keyboard.KeyBindingRepository; | ||
import org.jabref.gui.maintable.ColumnPreferences; | ||
import org.jabref.gui.maintable.MainTablePreferences; | ||
|
@@ -193,7 +194,8 @@ public class JabRefPreferences implements PreferencesService { | |
public static final String EDITOR_EMACS_KEYBINDINGS_REBIND_CA = "editorEMACSkeyBindingsRebindCA"; | ||
public static final String EDITOR_EMACS_KEYBINDINGS_REBIND_CF = "editorEMACSkeyBindingsRebindCF"; | ||
public static final String GROUPS_DEFAULT_FIELD = "groupsDefaultField"; | ||
public static final String GROUP_INTERSECT_SELECTIONS = "groupIntersectSelections"; | ||
public static final String GROUP_INTERSECT_UNION_VIEW_MODE = "groupIntersectUnionViewModes"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this can be private now, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ? |
||
|
||
public static final String KEYWORD_SEPARATOR = "groupKeywordSeparator"; | ||
public static final String AUTO_ASSIGN_GROUP = "autoAssignGroup"; | ||
public static final String LIST_OF_FILE_COLUMNS = "listOfFileColumns"; | ||
|
@@ -578,9 +580,9 @@ private JabRefPreferences() { | |
defaults.put(AUTOCOMPLETER_FIRST_LAST, Boolean.FALSE); // "Autocomplete names in 'Firstname Lastname' format only" | ||
defaults.put(AUTOCOMPLETER_LAST_FIRST, Boolean.FALSE); // "Autocomplete names in 'Lastname, Firstname' format only" | ||
defaults.put(AUTOCOMPLETER_COMPLETE_FIELDS, "author;editor;title;journal;publisher;keywords;crossref;related;entryset"); | ||
defaults.put(GROUP_INTERSECT_SELECTIONS, Boolean.FALSE); | ||
defaults.put(GROUPS_DEFAULT_FIELD, FieldName.KEYWORDS); | ||
defaults.put(AUTO_ASSIGN_GROUP, Boolean.TRUE); | ||
defaults.put(GROUP_INTERSECT_UNION_VIEW_MODE, GroupViewMode.INTERSECTION.name()); | ||
defaults.put(KEYWORD_SEPARATOR, ", "); | ||
defaults.put(TOOLBAR_VISIBLE, Boolean.TRUE); | ||
defaults.put(DEFAULT_ENCODING, StandardCharsets.UTF_8.name()); | ||
|
@@ -1756,4 +1758,12 @@ public void setWorkingDir(Path dir) { | |
put(WORKING_DIRECTORY, dir.toString()); | ||
|
||
} | ||
|
||
public GroupViewMode getGroupViewMode() { | ||
return GroupViewMode.valueOf(get(GROUP_INTERSECT_UNION_VIEW_MODE)); | ||
} | ||
|
||
public void setGroupViewMode(GroupViewMode mode) { | ||
put(GROUP_INTERSECT_UNION_VIEW_MODE, mode.name()); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make them private again