Quick Select Support for the GroupCreationTab #77
@ -0,0 +1,92 @@
|
|||||||
|
package envoy.client.ui.control;
|
||||||
|
|
||||||
|
import java.util.function.Consumer;
|
||||||
|
|
||||||
|
import javafx.geometry.*;
|
||||||
|
import javafx.scene.control.*;
|
||||||
|
import javafx.scene.image.ImageView;
|
||||||
|
import javafx.scene.layout.*;
|
||||||
|
import javafx.scene.shape.Rectangle;
|
||||||
|
|
||||||
|
import envoy.client.util.IconUtil;
|
||||||
|
import envoy.data.*;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Displays an {@link User} as a quick select control which is used in the
|
||||||
|
* quick select list.
|
||||||
|
*
|
||||||
|
* @author Maximilian Käfer
|
||||||
|
* @since Envoy Client v0.3-beta
|
||||||
|
*/
|
||||||
|
public class QuickSelectControl extends VBox {
|
||||||
|
|
||||||
|
private User user;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Creates an instance of the {@code QuickSelectControl}.
|
||||||
|
*
|
||||||
|
* @param user the contact whose data is used to create this instance.
|
||||||
|
* @param action the action to perform when a contact is removed with this control as a parameter
|
||||||
|
* @since Envoy Client v0.3-beta
|
||||||
|
*/
|
||||||
|
public QuickSelectControl(User user, Consumer<QuickSelectControl> action) {
|
||||||
|
this.user = user;
|
||||||
|
setPadding(new Insets(1, 0, 0, 0));
|
||||||
|
setPrefWidth(37);
|
||||||
|
setMaxWidth(37);
|
||||||
|
setMinWidth(37);
|
||||||
|
var stackPane = new StackPane();
|
||||||
|
stackPane.setAlignment(Pos.TOP_CENTER);
|
||||||
|
|
||||||
|
// Profile picture
|
||||||
|
var picHold = new VBox();
|
||||||
|
picHold.setPadding(new Insets(2, 0, 0, 0));
|
||||||
|
picHold.setPrefHeight(35);
|
||||||
|
picHold.setMaxHeight(35);
|
||||||
|
picHold.setMinHeight(35);
|
||||||
|
var contactProfilePic = new ImageView(IconUtil.loadIconThemeSensitive("user_icon", 32));
|
||||||
|
final var clip = new Rectangle();
|
||||||
|
clip.setWidth(32);
|
||||||
|
clip.setHeight(32);
|
||||||
|
clip.setArcHeight(32);
|
||||||
|
clip.setArcWidth(32);
|
||||||
mpk marked this conversation as resolved
|
|||||||
|
contactProfilePic.setClip(clip);
|
||||||
|
picHold.getChildren().add(contactProfilePic);
|
||||||
|
stackPane.getChildren().add(picHold);
|
||||||
|
|
||||||
|
var hBox = new HBox();
|
||||||
|
hBox.setPrefHeight(12);
|
||||||
|
hBox.setMaxHeight(12);
|
||||||
|
hBox.setMinHeight(12);
|
||||||
|
var region = new Region();
|
||||||
|
hBox.getChildren().add(region);
|
||||||
|
HBox.setHgrow(region, Priority.ALWAYS);
|
||||||
|
|
||||||
|
var removeBtn = new Button();
|
||||||
|
removeBtn.setPrefSize(12, 12);
|
||||||
|
removeBtn.setMaxSize(12, 12);
|
||||||
|
removeBtn.setMinSize(12, 12);
|
||||||
mpk marked this conversation as resolved
delvh
commented
As far as I can see, this is the only reason, why you're limiting this component to be used only in the GroupCreationTab. The OOP approach would be to define a setter that delegates what action to perform on the mouse click of the remove button. This would look something like this:
This additionally brings the advantage of reusability. When this is implemented, the As far as I can see, this is the only reason, why you're limiting this component to be used only in the GroupCreationTab.
The OOP approach would be to define a setter that delegates what action to perform on the mouse click of the remove button. This would look something like this:
```
public void onRemoveButtonClicked(BiConsumer<? super MouseEvent, User> action){
removeBtn.setOnMouseClicked(evt->action.accept(evt, user));
}
```
This additionally brings the advantage of reusability.
When this is implemented, the `GroupCreationTab` can be removed from the declaration of this component.
|
|||||||
|
removeBtn.setOnMouseClicked(evt -> action.accept(this));
|
||||||
|
removeBtn.setId("remove-button");
|
||||||
delvh
commented
If I understand you correctly, the only purpose of the I've experimented a bit and found out that I can achieve the same result using two components (and a few lines) less:
If I understand you correctly, the only purpose of the `HBox` is to position the remove button to the far right of the component.
I've experimented a bit and found out that I can achieve the same result using two components (and a few lines) less:
1. Remove lines 56-63 + 71 (`stackPane.getChildren(...)`)
2. Change `hBox` in line 70 to `picHold`
3. Make `picHold` from a `VBox` to an `HBox`
4. Done.
mpk
commented
Just paste your modified code an a comment because it is very hard to understand what you want if you just give a description. (Because the lines have changed) Just paste your modified code an a comment because it is very hard to understand what you want if you just give a description. (Because the lines have changed)
delvh
commented
Funny thing. They haven't. But still:
Funny thing. They haven't. But still:
1. Remove the variables `hBox` and `region`
2. the `picHold` variable should be an `HBox`, not a `VBox`
3. the line `hBox.getChildren().add(removeBtn);` should be
```
picHold.getChildren().add(removeBtn);
```
4. The line below where you are adding `hBox` to `stackPane` can be deleted
mpk
commented
I won't do this as it has no necessarity here. Btw I have the feeling that you did not understand the whole mechanism behind the component definition and usage. Alternatively phrased, I won't do it because no. Sincerely @kske aka Jesus, das Massiv I won't do this as it has no necessarity here. Btw I have the feeling that you did not understand the whole mechanism behind the component definition and usage.
Alternatively phrased, I won't do it because no. Sincerely @kske aka Jesus, das Massiv
delvh
commented
So why exactly are we now allowing unnecessary overhead? I've shown you in detail that I can reproduce the same result using two components less. What's the deal with that? So why exactly are we now allowing unnecessary overhead? I've shown you in detail that I can reproduce the same result using two components less. What's the deal with that?
delvh
commented
Oh, and by the way, if you have to resort to ' I won't do it because no' as the reasoning for something, then something is wrong with that thing. Oh, and by the way, if you have to resort to ' I won't do it because no' as the reasoning for something, then something is wrong with that thing.
|
|||||||
|
hBox.getChildren().add(removeBtn);
|
||||||
|
stackPane.getChildren().add(hBox);
|
||||||
|
getChildren().add(stackPane);
|
||||||
|
|
||||||
|
var nameLabel = new Label();
|
||||||
|
nameLabel.setPrefSize(35, 20);
|
||||||
|
nameLabel.setMaxSize(35, 20);
|
||||||
|
nameLabel.setMinSize(35, 20);
|
||||||
|
nameLabel.setText(user.getName());
|
||||||
|
nameLabel.setAlignment(Pos.TOP_CENTER);
|
||||||
mpk marked this conversation as resolved
delvh
commented
Unnecessary. Delete the line. Unnecessary. Delete the line.
mpk
commented
No it is defenitly not as this is centering the label! No it is defenitly not as this is centering the label!
|
|||||||
|
nameLabel.setPadding(new Insets(0, 5, 0, 0));
|
||||||
|
getChildren().add(nameLabel);
|
||||||
|
|
||||||
|
getStyleClass().add("quick-select");
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @return the user whose data is used in this instance
|
||||||
|
* @since Envoy Client v0.3-beta
|
||||||
|
*/
|
||||||
|
public User getUser() { return user; }
|
||||||
|
}
|
@ -7,11 +7,12 @@ import java.util.stream.Collectors;
|
|||||||
import javafx.application.Platform;
|
import javafx.application.Platform;
|
||||||
import javafx.fxml.FXML;
|
import javafx.fxml.FXML;
|
||||||
import javafx.scene.control.*;
|
import javafx.scene.control.*;
|
||||||
|
import javafx.scene.input.MouseEvent;
|
||||||
import javafx.scene.layout.HBox;
|
import javafx.scene.layout.HBox;
|
||||||
|
|
||||||
import envoy.client.data.*;
|
import envoy.client.data.*;
|
||||||
import envoy.client.event.BackEvent;
|
import envoy.client.event.BackEvent;
|
||||||
import envoy.client.ui.control.ContactControl;
|
import envoy.client.ui.control.*;
|
||||||
import envoy.client.ui.listcell.ListCellFactory;
|
import envoy.client.ui.listcell.ListCellFactory;
|
||||||
import envoy.data.*;
|
import envoy.data.*;
|
||||||
import envoy.event.GroupCreation;
|
import envoy.event.GroupCreation;
|
||||||
@ -58,6 +59,9 @@ public class GroupCreationTab implements EventListener {
|
|||||||
@FXML
|
@FXML
|
||||||
private HBox errorProceedBox;
|
private HBox errorProceedBox;
|
||||||
|
|
||||||
|
@FXML
|
||||||
|
private ListView<QuickSelectControl> quickSelectList;
|
||||||
|
|
||||||
private String name;
|
private String name;
|
||||||
|
|
||||||
private final LocalDB localDB = Context.getInstance().getLocalDB();
|
private final LocalDB localDB = Context.getInstance().getLocalDB();
|
||||||
@ -67,7 +71,6 @@ public class GroupCreationTab implements EventListener {
|
|||||||
@FXML
|
@FXML
|
||||||
private void initialize() {
|
private void initialize() {
|
||||||
userList.setCellFactory(new ListCellFactory<>(ContactControl::new));
|
userList.setCellFactory(new ListCellFactory<>(ContactControl::new));
|
||||||
userList.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
|
|
||||||
createButton.setDisable(true);
|
createButton.setDisable(true);
|
||||||
eventBus.registerListener(this);
|
eventBus.registerListener(this);
|
||||||
userList.getItems()
|
userList.getItems()
|
||||||
@ -78,6 +81,8 @@ public class GroupCreationTab implements EventListener {
|
|||||||
.filter(not(localDB.getUser()::equals))
|
.filter(not(localDB.getUser()::equals))
|
||||||
.map(User.class::cast)
|
.map(User.class::cast)
|
||||||
.collect(Collectors.toList()));
|
.collect(Collectors.toList()));
|
||||||
|
resizeQuickSelectSpace(0);
|
||||||
|
quickSelectList.addEventFilter(MouseEvent.MOUSE_PRESSED, evt -> evt.consume());
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -86,7 +91,15 @@ public class GroupCreationTab implements EventListener {
|
|||||||
* @since Envoy Client v0.1-beta
|
* @since Envoy Client v0.1-beta
|
||||||
*/
|
*/
|
||||||
@FXML
|
@FXML
|
||||||
private void userListClicked() { createButton.setDisable(userList.getSelectionModel().isEmpty() || groupNameField.getText().isBlank()); }
|
private void userListClicked() {
|
||||||
|
if (userList.getSelectionModel().getSelectedItem() != null) {
|
||||||
|
quickSelectList.getItems().add(new QuickSelectControl(userList.getSelectionModel().getSelectedItem(), this::removeFromQuickSelection));
|
||||||
|
createButton.setDisable(quickSelectList.getItems().isEmpty() || groupNameField.getText().isBlank());
|
||||||
|
resizeQuickSelectSpace(60);
|
||||||
|
userList.getItems().remove(userList.getSelectionModel().getSelectedItem());
|
||||||
|
userList.getSelectionModel().clearSelection();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Checks, whether the {@code createButton} can be enabled because text is
|
* Checks, whether the {@code createButton} can be enabled because text is
|
||||||
@ -95,7 +108,7 @@ public class GroupCreationTab implements EventListener {
|
|||||||
* @since Envoy Client v0.1-beta
|
* @since Envoy Client v0.1-beta
|
||||||
*/
|
*/
|
||||||
@FXML
|
@FXML
|
||||||
private void textUpdated() { createButton.setDisable(userList.getSelectionModel().isEmpty() || groupNameField.getText().isBlank()); }
|
private void textUpdated() { createButton.setDisable(quickSelectList.getItems().isEmpty() || groupNameField.getText().isBlank()); }
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Sends a {@link GroupCreation} to the server and closes this scene.
|
* Sends a {@link GroupCreation} to the server and closes this scene.
|
||||||
@ -123,6 +136,9 @@ public class GroupCreationTab implements EventListener {
|
|||||||
// Restoring the original design as tabs will always be reused
|
// Restoring the original design as tabs will always be reused
|
||||||
setErrorMessageLabelSize(0);
|
setErrorMessageLabelSize(0);
|
||||||
groupNameField.clear();
|
groupNameField.clear();
|
||||||
|
quickSelectList.getItems().forEach(q -> userList.getItems().add(q.getUser()));
|
||||||
|
quickSelectList.getItems().clear();
|
||||||
|
resizeQuickSelectSpace(0);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -136,7 +152,7 @@ public class GroupCreationTab implements EventListener {
|
|||||||
private void createGroup(String name) {
|
private void createGroup(String name) {
|
||||||
Context.getInstance()
|
Context.getInstance()
|
||||||
.getClient()
|
.getClient()
|
||||||
.send(new GroupCreation(name, userList.getSelectionModel().getSelectedItems().stream().map(User::getID).collect(Collectors.toSet())));
|
.send(new GroupCreation(name, quickSelectList.getItems().stream().map(q -> q.getUser().getID()).collect(Collectors.toSet())));
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -151,6 +167,27 @@ public class GroupCreationTab implements EventListener {
|
|||||||
return localDB.getChats().stream().map(Chat::getRecipient).filter(Group.class::isInstance).map(Contact::getName).anyMatch(newName::equals);
|
return localDB.getChats().stream().map(Chat::getRecipient).filter(Group.class::isInstance).map(Contact::getName).anyMatch(newName::equals);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Removes an element from the quickSelectList.
|
||||||
|
*
|
||||||
|
* @param element the element to be removed.
|
||||||
|
* @since Envoy Client v0.3-beta
|
||||||
|
*/
|
||||||
|
public void removeFromQuickSelection(QuickSelectControl element) {
|
||||||
|
quickSelectList.getItems().remove(element);
|
||||||
|
userList.getItems().add(element.getUser());
|
||||||
|
if (quickSelectList.getItems().isEmpty()) {
|
||||||
|
resizeQuickSelectSpace(0);
|
||||||
|
createButton.setDisable(true);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private void resizeQuickSelectSpace(int value) {
|
||||||
|
quickSelectList.setPrefHeight(value);
|
||||||
|
quickSelectList.setMaxHeight(value);
|
||||||
|
quickSelectList.setMinHeight(value);
|
||||||
|
}
|
||||||
|
|
||||||
@FXML
|
@FXML
|
||||||
private void backButtonClicked() {
|
private void backButtonClicked() {
|
||||||
eventBus.dispatch(new BackEvent());
|
eventBus.dispatch(new BackEvent());
|
||||||
|
@ -41,7 +41,7 @@
|
|||||||
-fx-scale-y: 1.05;
|
-fx-scale-y: 1.05;
|
||||||
}
|
}
|
||||||
|
|
||||||
.label {
|
.label, .quick-select {
|
||||||
-fx-background-color: transparent;
|
-fx-background-color: transparent;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -144,3 +144,15 @@
|
|||||||
visibility: hidden ;
|
visibility: hidden ;
|
||||||
-fx-padding: -20.0 0.0 0.0 0.0;
|
-fx-padding: -20.0 0.0 0.0 0.0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#quick-select-list .scroll-bar:horizontal{
|
||||||
|
-fx-pref-height: 0;
|
||||||
|
-fx-max-height: 0;
|
||||||
|
-fx-min-height: 0;
|
||||||
mpk marked this conversation as resolved
delvh
commented
Is that even needed in this case? Is that even needed in this case?
delvh
commented
Meant is only the last line... Meant is only the last line...
mpk
commented
Yes it is absolutly necessary. Yes it is absolutly necessary.
delvh
commented
Why? If the max height is already at 0, the pref height won't be larger than that, same for the min height... Why? If the max height is already at 0, the pref height won't be larger than that, same for the min height...
mpk
commented
No thats the thing I tried it and the scroll bars appear. I wondered to. No thats the thing I tried it and the scroll bars appear. I wondered to.
|
|||||||
|
}
|
||||||
|
|
||||||
|
#quick-select-list .scroll-bar:vertical{
|
||||||
|
-fx-pref-width: 0;
|
||||||
|
-fx-max-width: 0;
|
||||||
|
-fx-min-width: 0;
|
||||||
mpk marked this conversation as resolved
delvh
commented
Is that even needed in this case? Is that even needed in this case?
delvh
commented
Meant is only the last line... Meant is only the last line...
mpk
commented
Yes it is absolutly necessary. Yes it is absolutly necessary.
delvh
commented
Why? If the max width is already 0, not even the pref width should matter, especially not the min width. Why? If the max width is already 0, not even the pref width should matter, especially not the min width.
mpk
commented
No thats the thing I tried it and the scroll bars appear. I wondered to. No thats the thing I tried it and the scroll bars appear. I wondered to.
|
|||||||
|
}
|
||||||
|
@ -18,7 +18,7 @@
|
|||||||
-fx-background-color: lightgray;
|
-fx-background-color: lightgray;
|
||||||
}
|
}
|
||||||
|
|
||||||
#message-list, .text-field, .password-field, .tooltip, .pane, .pane .content, .vbox, .titled-pane > .title, .titled-pane > *.content, .context-menu, .menu-item {
|
#message-list, .text-field, .password-field, .tooltip, .pane, .pane .content, .vbox, .titled-pane > .title, .titled-pane > *.content, .context-menu, .menu-item, #quick-select-list {
|
||||||
-fx-background-color: #222222;
|
-fx-background-color: #222222;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -69,7 +69,7 @@
|
|||||||
-fx-background-color: transparent;
|
-fx-background-color: transparent;
|
||||||
}
|
}
|
||||||
|
|
||||||
.scroll-bar:vertical .increment-arrow, .scroll-bar:vertical .decrement-arrow {
|
.scroll-bar:vertical .increment-arrow, .scroll-bar:vertical .decrement-arrow, .list-cell {
|
||||||
-fx-background-color: transparent;
|
-fx-background-color: transparent;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -83,3 +83,8 @@
|
|||||||
-fx-text-fill: white;
|
-fx-text-fill: white;
|
||||||
-fx-background-color: transparent;
|
-fx-background-color: transparent;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#remove-button {
|
||||||
|
-fx-background-color: red;
|
||||||
|
-fx-background-radius: 1em;
|
||||||
|
}
|
||||||
|
@ -64,6 +64,7 @@
|
|||||||
<Insets bottom="5.0" top="5" />
|
<Insets bottom="5.0" top="5" />
|
||||||
</VBox.margin>
|
</VBox.margin>
|
||||||
</Label>
|
</Label>
|
||||||
|
<ListView fx:id="quickSelectList" id="quick-select-list" orientation="HORIZONTAL" prefHeight="60.0" />
|
||||||
<ListView id="chat-list" fx:id="userList" focusTraversable="false" onMouseClicked="#userListClicked" prefWidth="316.0" VBox.vgrow="ALWAYS">
|
<ListView id="chat-list" fx:id="userList" focusTraversable="false" onMouseClicked="#userListClicked" prefWidth="316.0" VBox.vgrow="ALWAYS">
|
||||||
<contextMenu>
|
<contextMenu>
|
||||||
<ContextMenu anchorLocation="CONTENT_TOP_LEFT" />
|
<ContextMenu anchorLocation="CONTENT_TOP_LEFT" />
|
||||||
|
This is, as far as I can see, unnecessary.
Have tested it, it is.