Add Ability to Change the User Status Freely #90
@ -1,12 +1,10 @@
|
||||
package envoy.client.event;
|
||||
|
||||
import envoy.data.User;
|
||||
import envoy.data.User.UserStatus;
|
||||
import envoy.event.Event;
|
||||
|
||||
/**
|
||||
* Conveys the action that the currently logged in {@link User} has changed his
|
||||
* status (manually).
|
||||
* Signifies a manual status change of the client user.
|
||||
*
|
||||
delvh marked this conversation as resolved
|
||||
* @author Leon Hofmeister
|
||||
* @since Envoy Client v0.3-beta
|
||||
@ -16,7 +14,7 @@ public class OwnStatusChange extends Event<UserStatus> {
|
||||
private static final long serialVersionUID = 1L;
|
||||
|
||||
/**
|
||||
* @param value the new user status of the currently logged in user
|
||||
* @param value the new user status of the client user
|
||||
* @since Envoy Client v0.3-beta
|
||||
*/
|
||||
public OwnStatusChange(UserStatus value) { super(value); }
|
||||
|
@ -22,8 +22,18 @@ public final class ShutdownHelper {
|
||||
*
|
||||
* @since Envoy Client v0.2-beta
|
||||
*/
|
||||
public static void exit() {
|
||||
if (Settings.getInstance().isHideOnClose() && StatusTrayIcon.isSupported()) Context.getInstance().getStage().setIconified(true);
|
||||
public static void exit() { exit(false); }
|
||||
|
||||
/**
|
||||
* Exits Envoy immediately if {@code force = true},
|
||||
* else it can exit or minimize Envoy, depending on the current state of
|
||||
* {@link Settings#isHideOnClose()} and {@link StatusTrayIcon#isSupported()}.
|
||||
*
|
||||
* @param force whether to close in any case.
|
||||
* @since Envoy Client v0.2-beta
|
||||
*/
|
||||
public static void exit(boolean force) {
|
||||
mpk marked this conversation as resolved
mpk
commented
Why exactly do you want the option of force closing the application if you hardcoded it to false when you are calling the method? Why exactly do you want the option of force closing the application if you hardcoded it to false when you are calling the method?
delvh
commented
Because otherwise we encounter a problem in Because otherwise we encounter a problem in `StatusTrayIcon` as I've noticed:
The previous method only checked whether `settings.isHideOnClose()` and `StatusTryIcon.isSupported()` are `true`. If a StatusTrayIcon is shown, both are automatically true. Hence, nothing would happen when you click on Exit in the TrayIcon (except that the stage would be minimized, which is not what is wanted here). That's the reason why this extra method was needed.
mpk
commented
OK than it is clear OK than it is clear
|
||||
if (!force && Settings.getInstance().isHideOnClose() && StatusTrayIcon.isSupported()) Context.getInstance().getStage().setIconified(true);
|
||||
else {
|
||||
EventBus.getInstance().dispatch(new EnvoyCloseEvent());
|
||||
System.exit(0);
|
||||
|
@ -6,6 +6,7 @@ import java.awt.TrayIcon.MessageType;
|
||||
import javafx.application.Platform;
|
||||
import javafx.stage.Stage;
|
||||
|
||||
import envoy.client.event.OwnStatusChange;
|
||||
import envoy.client.helper.ShutdownHelper;
|
||||
import envoy.client.util.*;
|
||||
import envoy.data.Message;
|
||||
@ -56,27 +57,28 @@ public final class StatusTrayIcon implements EventListener {
|
||||
|
||||
// Adding the exit menu item
|
||||
final var exitMenuItem = new MenuItem("Exit");
|
||||
exitMenuItem.addActionListener(evt -> ShutdownHelper.exit());
|
||||
exitMenuItem.addActionListener(evt -> ShutdownHelper.exit(true));
|
||||
popup.add(exitMenuItem);
|
||||
|
||||
// Adding the logout menu item
|
||||
final var logoutMenuItem = new MenuItem("Logout");
|
||||
logoutMenuItem.addActionListener(evt -> { hide(); UserUtil.logout(); });
|
||||
popup.add(exitMenuItem);
|
||||
logoutMenuItem.addActionListener(evt -> { hide(); Platform.runLater(UserUtil::logout); });
|
||||
delvh marked this conversation as resolved
Outdated
kske
commented
This appears to be an error. This appears to be an error.
delvh
commented
Ctrl C, Ctrl V. Ctrl C, Ctrl V.
|
||||
popup.add(logoutMenuItem);
|
||||
|
||||
// Adding the status change items
|
||||
final var statusSubMenu = new Menu("Change status");
|
||||
for (final var status : UserStatus.values()) {
|
||||
final var statusMenuItem = new MenuItem(status.toString().toLowerCase());
|
||||
statusMenuItem.addActionListener(evt -> UserUtil.changeStatus(status));
|
||||
statusMenuItem.addActionListener(evt -> Platform.runLater(() -> UserUtil.changeStatus(status)));
|
||||
statusSubMenu.add(statusMenuItem);
|
||||
}
|
||||
popup.add(statusSubMenu);
|
||||
|
||||
trayIcon.setPopupMenu(popup);
|
||||
|
||||
// Only display messages if the stage is not focused
|
||||
stage.focusedProperty().addListener((ov, onHidden, onShown) -> displayMessages = !ov.getValue());
|
||||
// Only display messages if the stage is not focused and the current user status
|
||||
// is not BUSY (if BUSY, displayMessages will be false)
|
||||
stage.focusedProperty().addListener((ov, wasFocused, isFocused) -> displayMessages = !displayMessages && wasFocused ? false : !isFocused);
|
||||
|
||||
// Show the window if the user clicks on the icon
|
||||
trayIcon.addActionListener(evt -> Platform.runLater(() -> { stage.setIconified(false); stage.toFront(); stage.requestFocus(); }));
|
||||
@ -103,6 +105,9 @@ public final class StatusTrayIcon implements EventListener {
|
||||
*/
|
||||
public void hide() { SystemTray.getSystemTray().remove(trayIcon); }
|
||||
|
||||
@Event
|
||||
delvh marked this conversation as resolved
Outdated
kske
commented
I think the point of the "busy" status is that the user does not want to be disturbed, so maybe put a check in place that prevents the notification in this status. Also, could we keep the previous formatting of this section? The new one looks very confusing to me. I think the point of the "busy" status is that the user does not want to be disturbed, so maybe put a check in place that prevents the notification in this status.
Also, could we keep the previous formatting of this section? The new one looks very confusing to me.
|
||||
private void onOwnStatusChange(OwnStatusChange statusChange) { displayMessages = !statusChange.get().equals(UserStatus.BUSY); }
|
||||
|
||||
@Event
|
||||
private void onMessage(Message message) {
|
||||
if (displayMessages) trayIcon
|
||||
|
@ -74,7 +74,7 @@ public final class ChatSceneCommands {
|
||||
alert.setContentText("Please provide an existing status");
|
||||
alert.showAndWait();
|
||||
}
|
||||
}).setDescription("Changes your status to the given status.").setNumberOfArguments(1).setDefaults("OFFLINE").build("status");
|
||||
}).setDescription("Changes your status to the given status.").setNumberOfArguments(1).setDefaults("").build("status");
|
||||
kske
commented
Any reason why the default is "offline"? This should be documented. Any reason why the default is "offline"? This should be documented.
delvh
commented
to 1. Well, normally when executing this command I'd argue that you want to change it, (from "ONLINE" in the standard case) and "OFFLINE" seemed like the most reasonable value. If you want another value, I can change that, i.e. BUSY? to 1. Well, normally when executing this command I'd argue that you want to change it, (from "ONLINE" in the standard case) and "OFFLINE" seemed like the most reasonable value. If you want another value, I can change that, i.e. BUSY?
to 2. Where should I document it? Our wiki is a laughable joke and the only location where it could be partially useful for the consumer would be inside the description of this system command. Should I add it there?
kske
commented
1. In my opinion, there is no sensible default. Rather, the parameter should be mandatory.
2. If you would put the documentation inside the wiki, it would become less of a joke. Maybe it's also possible to display the default values to the user in some way?
delvh
commented
1. Okay, I can remove the default value. Even though that completely defies the purpose of default values.
2. I've already thought about doing that (inspired by Skype)
but that'll require some major changes to the underlying system command system (=>another branch)
kske
commented
To my understanding, default values are only used if there is a default that actually makes sense. In case of status changes, I don't see, why a specific status would be the most frequently used one. To my understanding, default values are only used if there is a default that actually makes sense. In case of status changes, I don't see, why a specific status would be the most frequently used one.
delvh
commented
Have a look at the declaration again. Already fixed. Have a look at the declaration again. Already fixed.
|
||||
|
||||
// Selection of a new message initialization
|
||||
messageDependantAction("s",
|
||||
|
@ -194,7 +194,6 @@ public final class ChatScene implements EventListener, Restorable {
|
||||
settingsButton.setAlignment(Pos.BOTTOM_RIGHT);
|
||||
HBox.setHgrow(spaceBetweenUserAndSettingsButton, Priority.ALWAYS);
|
||||
delvh marked this conversation as resolved
delvh
commented
@DieGurke The region you requested is already here, but for some strange reason it doesn't work as expected. @DieGurke The region you requested is already here, but for some strange reason it doesn't work as expected.
Any idea why?
Feel free to take on that challenge, because I've tried it for about twenty minutes and could not get it to work.
mpk
commented
I could't fix it neither. Preferably you open an issue for that. I could't fix it neither. Preferably you open an issue for that.
|
||||
generateOwnStatusControl();
|
||||
System.out.println(ownContactControl.getChildren());
|
||||
|
||||
delvh marked this conversation as resolved
Outdated
kske
commented
This seems to be a leftover from development. This seems to be a leftover from development.
|
||||
Platform.runLater(() -> {
|
||||
final var online = client.isOnline();
|
||||
@ -270,9 +269,6 @@ public final class ChatScene implements EventListener, Restorable {
|
||||
});
|
||||
}
|
||||
|
||||
@Event(eventType = OwnStatusChange.class, priority = 50)
|
||||
private void onOwnStatusChange() { generateOwnStatusControl(); }
|
||||
|
||||
@Event
|
||||
private void onContactOperation(ContactOperation operation) {
|
||||
final var contact = operation.get();
|
||||
@ -379,6 +375,7 @@ public final class ChatScene implements EventListener, Restorable {
|
||||
recipientProfilePic.setImage(IconUtil.loadIconThemeSensitive("user_icon", 43));
|
||||
} else {
|
||||
topBarStatusLabel.setText(currentChat.getRecipient().getContacts().size() + " members");
|
||||
topBarStatusLabel.getStyleClass().clear();
|
||||
recipientProfilePic.setImage(IconUtil.loadIconThemeSensitive("group_icon", 43));
|
||||
}
|
||||
final var clip = new Rectangle();
|
||||
@ -719,6 +716,7 @@ public final class ChatScene implements EventListener, Restorable {
|
||||
attachmentView.setVisible(visible);
|
||||
}
|
||||
|
||||
@Event(eventType = OwnStatusChange.class, priority = 50)
|
||||
private void generateOwnStatusControl() {
|
||||
final var ownUserControl = new ContactControl(localDB.getUser());
|
||||
ownUserControl.setAlignment(Pos.CENTER_LEFT);
|
||||
delvh marked this conversation as resolved
kske
commented
You could annotate this as an event handler directly. You could annotate this as an event handler directly.
|
||||
|
@ -59,11 +59,7 @@ public final class GeneralSettingsPane extends SettingsPane {
|
||||
statusComboBox.getItems().setAll(UserStatus.values());
|
||||
statusComboBox.setValue(context.getLocalDB().getUser().getStatus());
|
||||
statusComboBox.setTooltip(new Tooltip("Change your current status"));
|
||||
statusComboBox.setOnAction(e -> {
|
||||
final var status = statusComboBox.getValue();
|
||||
if (status == null) return;
|
||||
else UserUtil.changeStatus(status);
|
||||
});
|
||||
statusComboBox.setOnAction(e -> UserUtil.changeStatus(statusComboBox.getValue()));
|
||||
getChildren().add(statusComboBox);
|
||||
|
||||
kske
commented
How is this possible? How is this possible?
delvh
commented
Artifact from when I called it using the selectionModel before stumbling upon this method. Artifact from when I called it using the selectionModel before stumbling upon this method.
Yes, agreed, their Javadoc doesn't mention anything about it,so I'll remove it.
|
||||
final var logoutButton = new Button("Logout");
|
||||
|
This construction is complicated even for my standards.
What about "Signifies a manual status change of the client user".