feat(focus): Add functionality to focus on running instance #13196
Conversation
![trag-bot[bot]](https://avatars.githubusercontent.com/in/856808?s=60&v=4){kind=small}
| @@ -140,25 +153,27 @@ private static boolean handleMultipleAppInstances(String[] args, RemotePreferenc | |||
| LOGGER.debug("Pinging other instance succeeded."); | |||
| if (args.length == 0) { | |||
| // There is already a server out there, avoid showing log "Passing arguments" while no arguments are provided. | |||
| LOGGER.warn("This JabRef instance is already running. Please switch to that instance."); | |||
| LOGGER.warn("A JabRef instance is already running. Switching to that instance."); | |||
There was a problem hiding this comment.
The log message uses an exclamation mark, which is discouraged as it can imply shouting. A period should be used instead to maintain a professional tone.
| /** | ||
| * Request the running instance to focus its window when a second instance tries to execute. No message content. | ||
| */ | ||
| FOCUS |
There was a problem hiding this comment.
The JavaDoc comment for the FOCUS constant is trivial and restates the obvious without adding new information or reasoning, which violates the guideline for non-trivial comments.
| @@ -3,4 +3,6 @@ | |||
| @FunctionalInterface | |||
| public interface RemoteMessageHandler { | |||
| void handleCommandLineArguments(String[] message); | |||
|
|
|||
| default void handleFocus() { } | |||
There was a problem hiding this comment.
The patch reformats the code without adding new statements, which violates the guideline that code should not be reformatted only because of syntax.
| * | ||
| * @return true if successful, false otherwise. | ||
| */ | ||
| public boolean sendFocus() { |
There was a problem hiding this comment.
The new public method 'sendFocus' should not return 'null'. It should use 'java.util.Optional' to handle cases where a return value might be absent.
| mainStage.toFront(); | ||
| mainStage.requestFocus(); |
There was a problem hiding this comment.
@InAnYan I think, it is either or here.
VS.Code just lets blink the app in the task bar (which is requestFocus IMHO). Here, the current instance is brought to front. I assume that requestFocus is made in addition to ensure it works?
There was a problem hiding this comment.
I removed the requestFocus statement - and it worked, too.
There was a problem hiding this comment.
@miguelro20 Please add a CHANGELOG.md entry. Please remove mainStage.requestFocus();, because ...toFront is enough (if you have other reasons, please share)
Then, this is good to go IMHO
|
@trag-bot didn't find any issues in the code! ✅✨ |
|
@koppor I believe I've made all the necessary changes, you were right about the requestFocus, it was redundant |
Closes #13129
This issue consisted on focusing a running instance of Jabref when the user tried to open another instance of the program. To do so, the first step was to modify the handleMultipleAppInstances function, to return an option from an enum containing CONTINUE, SHUTDOWN, FOCUS to determine what to do next. If there is no instance, CONTINUE is selected and one boots up, if an instance is running, FOCUS is returned. Focus is a function created inside UiCommands. In the launcher if FOCUS is returned it triggers RemoteClient's sendFocus function, which sends the message FOCUS and its outlined inside RemoteMessage. Then the RemoteListenerServer picks up this FOCUS message inside of a case within a switch, which calls a function from the RemoteMessageHandler called handlerFocus() and being overridden inside CLIMessageHandler, and sends the platform a list of UiCommands, consisting of one UiCommand.Focus (This because JabRefFrame expects a list of UiCommands not just one UiCommand). This UiCommand is then picked up in JabRefFrame, which then calls the toFront() and requestFocus() functions to show the running instance.
This makes the user see the already running instance, and at the same time handles all the functions needed to send Focus from the Launcher to the JabRefFrame.
Running JabRef Instance from IntelliJ
Message when running ./gradlew jabgui:run in Terminal with running instance
Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if change is visible to the user)