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 for issue 4629 #5150

Merged
merged 8 commits into from
Aug 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/main/java/org/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -1240,6 +1240,11 @@ public EditAction(Actions command) {
this.command = command;
}

@Override
public String toString() {
return this.command.toString();
}

@Override
public void execute() {
Node focusOwner = mainStage.getScene().getFocusOwner();
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/org/jabref/gui/actions/ActionFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ private static Label getAssociatedNode(MenuItem menuItem) {
}

public MenuItem configureMenuItem(Action action, Command command, MenuItem menuItem) {
ActionUtils.configureMenuItem(new JabRefAction(action, command, keyBindingRepository), menuItem);
ActionUtils.configureMenuItem(new JabRefAction(action, command, keyBindingRepository, Sources.FromMenu), menuItem);
setGraphic(menuItem, action);

// Show tooltips
Expand Down Expand Up @@ -105,7 +105,7 @@ public MenuItem createMenuItem(Action action, Command command) {
}

public CheckMenuItem createCheckMenuItem(Action action, Command command, boolean selected) {
CheckMenuItem checkMenuItem = ActionUtils.createCheckMenuItem(new JabRefAction(action, command, keyBindingRepository));
CheckMenuItem checkMenuItem = ActionUtils.createCheckMenuItem(new JabRefAction(action, command, keyBindingRepository, Sources.FromMenu));
checkMenuItem.setSelected(selected);
setGraphic(checkMenuItem, action);

Expand All @@ -127,7 +127,7 @@ public Menu createSubMenu(Action action, MenuItem... children) {
}

public Button createIconButton(Action action, Command command) {
Button button = ActionUtils.createButton(new JabRefAction(action, command, keyBindingRepository), ActionUtils.ActionTextBehavior.HIDE);
Button button = ActionUtils.createButton(new JabRefAction(action, command, keyBindingRepository, Sources.FromButton), ActionUtils.ActionTextBehavior.HIDE);

button.getStyleClass().setAll("icon-button");

Expand All @@ -140,7 +140,7 @@ public Button createIconButton(Action action, Command command) {

public ButtonBase configureIconButton(Action action, Command command, ButtonBase button) {
ActionUtils.configureButton(
new JabRefAction(action, command, keyBindingRepository),
new JabRefAction(action, command, keyBindingRepository, Sources.FromButton),
button,
ActionUtils.ActionTextBehavior.HIDE);

Expand Down
32 changes: 30 additions & 2 deletions src/main/java/org/jabref/gui/actions/JabRefAction.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package org.jabref.gui.actions;

import java.util.HashMap;
import java.util.Map;

import javafx.beans.binding.Bindings;

import org.jabref.Globals;
Expand All @@ -23,11 +26,23 @@ public JabRefAction(Action action, KeyBindingRepository keyBindingRepository) {
}

public JabRefAction(Action action, Command command, KeyBindingRepository keyBindingRepository) {
this(action, command, keyBindingRepository, null);
}

/**
* especially for the track execute when the action run the same function but from different source.
* @param source is a string contains the source, for example "button"
*/
public JabRefAction(Action action, Command command, KeyBindingRepository keyBindingRepository, Sources source) {
this(action, keyBindingRepository);

setEventHandler(event -> {
command.execute();
trackExecute(getActionName(action, command));
if (source == null) {
trackExecute(getActionName(action, command));
} else {
trackUserActionSource(getActionName(action, command), source);
}
});

disabledProperty().bind(command.executableProperty().not());
Expand All @@ -42,12 +57,25 @@ private String getActionName(Action action, Command command) {
if (command.getClass().isAnonymousClass()) {
return action.getText();
} else {
return command.getClass().getSimpleName();
String commandName = command.getClass().getSimpleName();
if ((command instanceof OldDatabaseCommandWrapper) || (command instanceof OldCommandWrapper) || commandName.contains("EditAction")) {
return command.toString();
} else {
return commandName;
}
Copy link
Member

Choose a reason for hiding this comment

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

I have been testing this method and there is a small bug in some EditAction cases (it shows org.jabref.gui.entryeditor.SourceTab$EditAction@56048e40FromMenu). Due to there are still many classes that use Actions and OldDatabaseCommandWrapper, it is a bit complicated to deal with all of them, but I think that something like this should work...

} else {
    String commandName = command.getClass().getSimpleName();
    if (command instanceof OldDatabaseCommandWrapper || commandName.equals("EditAction")) {
        return action.toString();
    } else {
        return commandName;
    }
}

Just a proposal, please adapt it in your own way :"D

}
}

private void trackExecute(String actionName) {
Globals.getTelemetryClient()
.ifPresent(telemetryClient -> telemetryClient.trackEvent(actionName));
}

private void trackUserActionSource(String actionName, Sources source) {
Map<String, String> properties = new HashMap<>();
Map<String, Double> measurements = new HashMap<>();
properties.put("Source", source.toString());

Globals.getTelemetryClient().ifPresent(telemetryClient -> telemetryClient.trackEvent(actionName, properties, measurements));
}
}
5 changes: 5 additions & 0 deletions src/main/java/org/jabref/gui/actions/OldCommandWrapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,9 @@ public ReadOnlyDoubleProperty progressProperty() {
public void setExecutable(boolean executable) {
this.executable.bind(BindingsHelper.constantOf(executable));
}

@Override
public String toString() {
return this.command.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ public double getProgress() {
return 0;
}

@Override
public String toString() {
return this.command.toString();
}

@Override
public ReadOnlyDoubleProperty progressProperty() {
return null;
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/org/jabref/gui/actions/Sources.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package org.jabref.gui.actions;

public enum Sources {
FromButton,
FromMenu

}