`Menu > Boards > New` should restore current board if save is aborted #1460 #1485

xinan · 2016-04-05T18:45:12Z

Now it will prompt for save if the current board is dirty, instead of alerting all the time but does not provide option to save.
It also does not clear the current board before the New action is confirmed. So it is fine to cancel a New action.
Instead of opening an empty board without any panels. Boards > New now opens up a fresh board with a single empty panel. (Default panel name and no filter.)

HansNewbie · 2016-04-06T08:38:52Z

@xinan no worries, close & reopen to trigger travis

HansNewbie · 2016-04-06T08:49:52Z

src/main/java/ui/ConfirmChangesDialog.java

+                                               + "Do you want to save them?";
+
+    public ConfirmChangesDialog(Stage stage) {
+        super(AlertType.CONFIRMATION);


We will need to merge this whole class to Util.DialogMessage. Either use the existing Warning dialog (not recommended) or create a new confirmation dialog in that class. We would like any dialog to be available for general use.

Why use of the existing showYesNoWarningDialog is not recommended?

Since you use confirmation type here. I think there will be some UI
difference, also. Unless if your dialog is also a warning.
On Apr 6, 2016 5:24 PM, "Liu Xinan" notifications@github.com wrote:

In src/main/java/ui/ConfirmChangesDialog.java
#1485 (comment):

+import javafx.scene.control.ButtonType;
+import javafx.stage.Stage;
+
+/**

* This class represents a confirm dialog that asks the user whether or not to save the current board before

* switching to another board, or creating a new board.

*/
+public class ConfirmChangesDialog extends Alert {
+

private static final String DIALOG_TITLE = "Save Changes?";

private static final String DIALOG_HEADER = "There are unsaved changes to your current board.";

private static final String DIALOG_CONTENT = "If you choose not to save, all unsaved changes will be discarded.\n\n"

+ "Do you want to save them?";

public ConfirmChangesDialog(Stage stage) {

super(AlertType.CONFIRMATION);

Why use of the existing showYesNoWarningDialog is not recommended?

—
You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub
https://github.com/HubTurbo/HubTurbo/pull/1485/files/e736de8ed9249ffc9ffb1cc5cd3c21adfcaf0563#r58675841

HansNewbie · 2016-04-07T13:17:35Z

src/main/java/ui/MenuControl.java

-    private void onBoardNew() {
-        logger.info("Menu: Boards > New");
-        newBoard();
+    public final boolean isCurrentOpenBoardDirty() {


I think isCurrentBoardDirty is sufficient as we only have 1 open board. Update header comment as well.

HansNewbie · 2016-04-07T13:38:20Z

Mostly cosmetic change.

1 thing needed: test for the confirmation dialog. Look at guitests>DialogMessageTests.

HansNewbie · 2016-04-08T05:30:52Z

Update me when ready for another review.

xinan · 2016-04-08T05:32:27Z

@HansNewbie Yes, it is ready!

HansNewbie · 2016-04-08T05:36:11Z

src/main/java/ui/MenuControl.java

-     * Returns a boolean indicating whether we can proceed with
-     * the new board creation. If there are no panels open, it
-     * returns true; else a dialog is shown for the user to confirm.
+     * Prompts a {@link BoardNameDialog} to ask the user for a new board name


Update comment to ... for a board name.

HansNewbie · 2016-04-08T05:36:45Z

src/main/java/ui/MenuControl.java

-            return true;
+    public final Optional<String> promptForBoardName() {
+        BoardNameDialog dlg = new BoardNameDialog(prefs, mainStage);
+        Optional<String> response = dlg.showAndWait();


Can we do return dlg.showAndWait()?

damithc · 2016-04-08T13:56:33Z

src/main/java/ui/MenuControl.java

    }

    /**
-     * Creates an empty board. User will be prompted to
-     * confirm the action if there are unclosed panels
+     * Prompts a dialog to ask the user whether or not to save the current board.


The method is doing more than just prompting. Both method name and comment misleading.

I can't think of a better name. Do you have any suggestion how to name such methods?

We can save after the prompt? Don't save in this method, I mean.

So I make it return a boolean?

That's what I have in mind.

damithc · 2016-04-09T07:36:09Z

src/main/java/ui/MenuControl.java

-            logger.info("New board creation cancelled");
-            return;
-        }
+    public final boolean promptToSaveCurrentBoard() {


A better name is isUserAgreeableToSavingBorad()

damithc · 2016-04-09T08:12:25Z

@HansNewbie any more comments?

HansNewbie · 2016-04-09T08:15:18Z

Looks good. No more comments.

xinan added the status.toReview label Apr 5, 2016

xinan force-pushed the 1460-fix-board-new branch from cac70c8 to e736de8 Compare April 5, 2016 19:03

ndt93 assigned HansNewbie Apr 6, 2016

HansNewbie closed this Apr 6, 2016

HansNewbie reopened this Apr 6, 2016

HansNewbie reviewed Apr 6, 2016
View reviewed changes

HansNewbie added status.ongoing and removed status.toReview labels Apr 6, 2016

xinan added status.toReview and removed status.ongoing labels Apr 6, 2016

HansNewbie reviewed Apr 7, 2016
View reviewed changes

xinan force-pushed the 1460-fix-board-new branch from b5b74cc to a5f3e70 Compare April 7, 2016 14:43

HansNewbie reviewed Apr 8, 2016
View reviewed changes

xinan force-pushed the 1460-fix-board-new branch from a5f3e70 to 7b0a8d2 Compare April 8, 2016 12:31

xinan mentioned this pull request Apr 8, 2016

Prompt user to save current board if there are unsaved changes #1199 #1224

Merged

damithc reviewed Apr 8, 2016
View reviewed changes

xinan force-pushed the 1460-fix-board-new branch from 7b0a8d2 to a89e60b Compare April 9, 2016 06:14

xinan added 9 commits April 9, 2016 14:24

Refactor "Boards > New"

3122240

Update BoardTests

a456d7b

Minor refactoring in MenuControl

03af651

Merge ConfirmChangesDialog into DialogMessage

bbf050c

Some cosmetic changes

36dbfb0

Add test for DialogMessage.showYesNoConfirmationDialog

8a91a41

Some more cosmetic changes

d310f4e

Save board outside prompting method

7b260f2

Make the DialogMessageTest clearer

50c0559

xinan force-pushed the 1460-fix-board-new branch from a89e60b to 50c0559 Compare April 9, 2016 06:34

damithc reviewed Apr 9, 2016
View reviewed changes

Rename promptToSaveCurrentBoard

70af3b8

HansNewbie added status.toMerge and removed status.toReview labels Apr 9, 2016

damithc added status.mergeApproved and removed status.toMerge labels Apr 9, 2016

xinan merged commit 70af3b8 into HubTurbo:master Apr 9, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`Menu > Boards > New` should restore current board if save is aborted #1460 #1485

`Menu > Boards > New` should restore current board if save is aborted #1460 #1485

xinan commented Apr 5, 2016

HansNewbie commented Apr 6, 2016

HansNewbie Apr 6, 2016

xinan Apr 6, 2016

HansNewbie Apr 6, 2016

HansNewbie Apr 7, 2016

HansNewbie commented Apr 7, 2016

HansNewbie commented Apr 8, 2016

xinan commented Apr 8, 2016

HansNewbie Apr 8, 2016

HansNewbie Apr 8, 2016

damithc Apr 8, 2016

xinan Apr 8, 2016

HansNewbie Apr 8, 2016

xinan Apr 9, 2016

HansNewbie Apr 9, 2016

damithc Apr 9, 2016

damithc commented Apr 9, 2016

HansNewbie commented Apr 9, 2016

Menu > Boards > New should restore current board if save is aborted #1460 #1485

Menu > Boards > New should restore current board if save is aborted #1460 #1485

Conversation

xinan commented Apr 5, 2016

HansNewbie commented Apr 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HansNewbie commented Apr 7, 2016

HansNewbie commented Apr 8, 2016

xinan commented Apr 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damithc commented Apr 9, 2016

HansNewbie commented Apr 9, 2016

`Menu > Boards > New` should restore current board if save is aborted #1460 #1485

`Menu > Boards > New` should restore current board if save is aborted #1460 #1485