Extract DialogHelper from Contacts Example to mvvmfx core

[sialcasa]

This is a pretty good helper and we should document a best practice how to open / close dialogs in mvvmfx

[mainrs]

I took a look at the DialogHelper. Here is my opinion:

• I really like the idea of the visible property. It could be used to show some progress dialogue with a progress bar based on a condition (loading some data that is required by an application to proceed). But I am not sure if this is really needed. As a developer you should normally try to load data on another thread than the UI thread so the application still responds to other user input (triggering other actions). If the data that needs to be loaded is really required to proceed than this method would be really good to have, if not, a statusbar would be nicer to have with a scope focused on updating it. Personally, I would keep it but do some modifications. Making sure any ui code is called on the dedicated thread should be taken care of.
• This class isn't an api too. Making it fluent shouldn't be hard but features need to be discussed and a list should be created to get an overview.
• This utility class still needs somehow to get a reference to the primary stage from the caller. But as far as I am aware of, there is no real way to make this happen. ViewModel and View are created by the framework, thus the primary stage can't be passed as a contractor argument (which would be bad design). Making it public static in the application class is a solution but I wouldn't prefer it. It breaks OOP and MVVM design principles which shouldn't be done if the framework is designed to implement MVVM in JavaFX.

Edit: What about providing a Scope for the primary stage? That would enable views to get a reference to it for calling the API. But it would also allow ViewModels to get the same scope. Normally one wouldn't do that though.

[manuel-mauky]

Getting a reference to the stage isn't a problem. Every UI control has a getScene() method. From the scene you can use getWindow() to get a window reference which is needed for the dialog.

See this example:

<?xml version="1.0" encoding="UTF-8"?>
<VBox fx:id="root" fx:controller="example.MyView">
</VBox>

public class MyView implements FxmlView<MyViewModel> {
    @FXML
    private VBox root;

    public void someMethod() {
        Scene scene = root.getScene();
        Window window = scene.getWindow();
        Stage dialogStage = new Stage();
        dialogStage.initModality(Modality.WINDOW_MODAL); // APPLICATION_MODAL is also possible
        dialogStage.initOwner(window);

        dialogStage.setScene(...);
        dialogStage.show();
    }
}

Getting a reference to the stage/window isn't a problem we as framework need to take care of. The DialogHelper in the contacts-example already has a method that takes a parentStage as argument. This has to be changed to type Window. After that it can be used to create modal dialogs.

[mainrs]

Oh, missed that one 😄 OK, mind if I prototype something and post it here? Trying to make a fluent api.

[manuel-mauky]

Ok, but don't bother too much with the Fluent API in the first place. The first thing we need is a method that takes all parameters that may be used and make that production ready. Creating a Fluent API afterwards isn't hard. As far as I can see we only need an additional argument in the initDialog method for the Title?

[mainrs]

I agree. Still relatively new to git. I need to fork, create a new branch and work on that branch, am I right?

[manuel-mauky]

Yes.

1. create a fork of mvvmFX in your github profile
2. create a branch from develop. Name it for example "331_dialog_helper" (we typically put the issue number in the branch name so it's easier to find it afterwards)
3. Write code and commit, push to your repo
4. Create a Pull-Request or post a comment here so I can look at your code first and give you feedback

[mainrs]

Here you go: https://github.com/SirWindfield/mvvmFX/tree/331_prototype_dialoghelper

[manuel-mauky]

I've looked into your code and found some things that I'd like to comment. If you create a PullRequest I can comment directly in the code.

[sialcasa]

one thing we should think about is to use the JavaFX Dialog API instead of creating plain Stages https://docs.oracle.com/javase/8/javafx/api/javafx/scene/control/Dialog.html

[mainrs]

one thing we should think about is to use the JavaFX Dialog API instead of creating plain Stages https://docs.oracle.com/javase/8/javafx/api/javafx/scene/control/Dialog.html

That would only apply to simple dialogues, am I right? If you, for example, want to show a new window for editing a contact within an address book, the dialogue class wouldn't be the right choice here. Correct me if I am wrong.

[sialcasa]

This is true but the name of the Class "DialogHelper" suggests more then only opening a new window. Maybe another name would be better?

[mainrs]

Ye, I just used the old one. A name that would fit better is probably WindowHelper.

[gtnarg]

For simple dialogs (or where the JavaFx Dialog API) is leveraged, could the Dialog not implement JavaView directly and be loaded by the ViewLoader (currently only supports Parent)? Then you could simply load the dialog and and call #showAndWait.

[mainrs]

For simple dialogs (or where the JavaFx Dialog API) is leveraged, could the Dialog not implement JavaView directly and be loaded by the ViewLoader (currently only supports Parent)? Then you could simply load the dialog and and call #showAndWait.

Don't want to be mean but I do not understand what you said. Would you mind explaining it in more detail to me or provide a really small code snippet?

[gtnarg]

Sorry, don't mean to high jack this thread, it probably doesn't apply to your helper which is useful for opening windows that are not javafx dialogs. When @sialcasa mentioned the JavaFx Dialog API, there are use cases where a dialog could be loaded directly as a view if the loader supported it (no extra extra helper class needed).

[manuel-mauky]

@gtnarg you are welcome to discuss with us :-) I think the hint for the official dialogs API is an important thing to keep in mind. In my oppinion there are 2 separate questions here:

1. How do you create simple alerts and messages with the official DIalogs API with mvvmFX. In the first place we should provide examples and documentation. Maybe we don't even need own helper classes for this.
2. How do you create complex dialogs/windows with mvvmFX. I think this is the question we are addressing here with the DialogHelper/WindowHelper. Maybe we can even use the official dialogs API here?

[mainrs]

How do you create complex dialogs/windows with mvvmFX. I think this is the question we are addressing here with the DialogHelper/WindowHelper. Maybe we can even use the official dialogs API here?

Do you mean the Alert class? Because this one is (in my opinion) is designed to be used for the old JOptionPane cases. I do not think that using it fits the problem here. Even the Dialog class provided by JavaFX goes into that direction by providing properties for header, text and and multiple buttons. It would provide properties to the user that do not have any effect on the outcome, for example the headerproperty.

[manuel-mauky]

Alert class is a subclass of Dialog. In my current project we are using the official Dialog class (and subsclasses of it) only for simple messages or to get a single input value from the user. For complex dialogs with whole forms in it we create new Stages similar to the approach we are discussing here. But: Maybe we can use/extend the official Dialog API even for such use cases. I don't know yet because I haven't tried. I don't know if there would be any advantages. The only thing I'd like to say is that we should keep this possibility in mind. Maybe there are advantages that we would lose otherwise.

[sialcasa]

I like the showAndWait option of the dialog api

[mainrs]

Dialog Closing Rules

It is important to understand what happens when a Dialog is closed, and also how a Dialog can be closed, especially in abnormal closing situations (such as when the 'X' button is clicked in a dialogs title bar, or when operating system specific keyboard shortcuts (such as alt-F4 on Windows) are entered). Fortunately, the outcome is well-defined in these situations, and can be best summarised in the following bullet points:

• JavaFX dialogs can only be closed 'abnormally' (as defined above) in two situations:
  1. When the dialog only has one button, or
  2. When the dialog has multiple buttons, as long as one of them meets one of the following requirements:
     1. The button has a ButtonType whose ButtonBar.ButtonData is of type ButtonBar.ButtonData.CANCEL_CLOSE.
     2. The button has a ButtonType whose ButtonBar.ButtonData returns true when ButtonBar.ButtonData.isCancelButton() is called.
• In all other situations, the dialog will refuse to respond to all close requests, remaining open until the user clicks on one of the available buttons in the DialogPane area of the dialog.
• If a dialog is closed abnormally, and if the dialog contains a button which meets one of the two criteria above, the dialog will attempt to set the result property to whatever value is returned from calling the result converter with the first matching ButtonType.
• If for any reason the result converter returns null, or if the dialog is closed when only one non-cancel button is present, the result property will be null, and the showAndWait() method will return Optional.empty(). This later point means that, if you use either of option 2 or option 3 (as presented earlier in this class documentation), the Optional.ifPresent(java.util.function.Consumer) lambda will never be called, and code will continue executing as if the dialog had not returned any value at all.

This would mean that the user needs to define his buttons using the official Dialog API, otherwise things won't work correctly. The problem that we face here is that there is no way to define those in the actual View that gets displayed. The View has no way to get the needed information (which button has been pressed) without any boilerplate code (it is probably possible to use the EventBus here). Defining what happens and which buttons to display will be then handled by the class that creates the View and not the View itself. This seems quite ugly to be honest. Would like to here some feedback concerning this.

I like the showAndWait option of the dialog api

I don't want to be mean by just rejecting all the stuff you guys say. This method is definitely nice. But the same outcome can be achieved by creating a Stage and setting its Modality to Modality.APPLICATION_MODAL. This would block all Events from reaching its Parent view, meaning the user needs to interact with the Window shown before he can do other stuff.

[manuel-mauky]

In the DialogHelper the openProperty will be set to false when the dialog is closed so it's possible to react on this event. I don't know if there are any special situations were the existing mechanism doesn't work. It's using the Stage.setOnCloseRequest callback. Maybe Stage.setOnHidden would be a better place for this.

At the moment I'm not sure what the best way of handling the buttons of a dialog in a mvvmFX-like way is. I will play around with the API a little bit. From my experience I can tell that it's typically the best way to first implement a clean version of handling something even if this includes a lot of boilerplate. When you have a clean and solid solution you can try to extract the boilerplate into helper classes or other utilities. This should be done for the Dialogs-API too.

In my oppinion the good thing about the showAndWait method isn't acutally that it prevents user input in other places like APPLICATION_MODAL but that the API is communicating in a clean way that this method will block until the user has finished interacting with the Dialog. When using the normal show method you have to take care for getting the user input asynchronously even if the whole process (showing a dialog, waiting till the user entered some values and pressed OK) is totally synchronous.

[mainrs]

When using the normal show method you have to take care for getting the user input asynchronously even if the whole process (showing a dialog, waiting till the user entered some values and pressed OK) is totally synchronous.

I agree with you. It might be better. Just a small question on the quote above: The user input should be handled the same way each time, independent of which method will finally be used. The View binds its properties to the ViewModel. That will be done for either of the solutions described above. I'm probably just understanding it wrong though...

[manuel-mauky]

I think it depends on the use case and there are at least 2 aspects here:

1. How is the dialog implemented internally?
2. How is the dialog used from a mvvmFX view/viewModel?

Even if the dialog is internally created with mvvmFX view you could hide this implementation detail from the user (the developer who is using the dialog) by providing the showAndWait semantics. Think of a dialog like in the contacts example: The dialog shows a "Create/Edit User Form" that is implemented with mvvmFX. It could return a Optional<User> from a showAndWait method. Or simply a boolean that indicates if the process was successful or not.

But on the other hand the semantics with the openProperty that controls if the dialog is open or not (like in the DialogHelper) has it's advantages too. At the moment I'm undecided which is better or if we may even could support both?

[mainrs]

The more we discuss here, the more I like the approach of the showAndWait method. Returning an Optional<T> would be nice to have. But the more complex I make the examples that could be used here the more hard it seems to realize. What about a setup wizard? Pressing the next button should somehow change the content of the pane. Returning some Optional<T> would mean that each time the user clicks next, a return value is shown. The developer needs, in this example, a way to hook up certain things (for example such buttons as next and back) and be able to have conditions on how a finish button can be used. It seems that this solution may be quite difficult to realize but probably not impossible to do. Hopefully you understand what I was trying to explain.

Edit: Supporting both would be possible by just creating two utility classes or just different methods and writing some javadoc for them.