Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

undo/redo/paste keyboard shortcuts #184

Closed
djskrien opened this issue Sep 27, 2015 · 11 comments
Closed

undo/redo/paste keyboard shortcuts #184

djskrien opened this issue Sep 27, 2015 · 11 comments

Comments

@djskrien
Copy link

I like the fact that so many operations in StyledTextAreas have keyboard shortcuts. However, three of the shortcuts are causing me problems, namely the shortcuts for undo, redo, and paste.

I wanted my application to have an Edit menu with menu items for each of these 3 operations (among others) and I wanted these menu items to display their usual keyboard shortcuts. So, for example, I added a Paste menu item and gave it the usual Ctrl+V (or Cmd+V on a Mac) shortcut. I then set the action of the Paste menu item to be the paste() method in ClipboardActions. Now, if the user ignores the keyboard shortcut and selects the Paste menu item with their mouse, then pasting works fine. However, if the user presses the keyboard shortcut for Paste, the paste is performed twice: once by the Menu item and once by the StyledTextArea. The same problem occurs with my Undo and Redo menu items.

One solution to this problem is to subclass StyledTextArea to turn off its keyboard shortcut so that the Paste menu item's shortcut is the only one that is active. Is this the best solution? If so, how do I turn off the Ctrl+V (and Cmd+V) shortcut in StyledTextArea? Your documentation explains how to add keyboard shortcuts but I didn't see any explanation of how to remove them.

@TomasMikula
Copy link
Member

I don't know whether the corresponding KEY_PRESSED event is consumed automatically when MenuItem's accelerator is triggered. The documentation does not say anything about that. But it seems that it doesn't. Also, have you tried consuming the MenuItem's ActionEvent? (Not sure if it helps...)

With the current version, you cannot ignore a keyboard shortcut, but you can override it to do nothing and consume the event:

EventHandlerHelper.install(
        area.onKeyPressedProperty(),
        EventHandlerHelper.on(keyPressed(V, SHORTCUT_DOWN)).act(e -> {}).create());

@JFormDesigner
Copy link
Contributor

Strange to hear that actions are performed twice for @djskrien because I had the opposide effect when developing Markdown Writer FX: When the a RichTextFX control is focused, it consumes all key events and the menu accelerators do not work

In Markdown Writer FX I workaround this by installing all menu accelarators into the RichTextFX controls. See: MarkdownEditorPane.java#L130-L132 and MainWindow.java#L302-L327

Made a little test app that demonstrates that menu accelarators do not work when a RichTextFX control has focus, but do work when a plain JavaFX text area has focus:

import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.control.*;
import javafx.scene.input.KeyCombination;
import javafx.scene.layout.BorderPane;
import javafx.stage.Stage;
import org.fxmisc.richtext.StyleClassedTextArea;

public class RichTextWithMenuBarTest extends Application {
    public static void main(String[] args) {
        launch(args);
    }

    @Override
    public void start(Stage primaryStage) {
        StyleClassedTextArea richTextArea = new StyleClassedTextArea();
        richTextArea.replaceText("RichTextFX control - Ctrl+O does not work when focused");

        TextArea textArea = new TextArea("TextArea control - Ctrl+O works when focused");

        BorderPane root = new BorderPane(richTextArea);
        root.setTop(createMenuBar());
        root.setBottom(textArea);

        Scene scene = new Scene(root, 600, 400);
        primaryStage.setScene(scene);
        primaryStage.setTitle("RichText With MenuBar Test");
        primaryStage.show();
    }

    private MenuBar createMenuBar() {
        MenuItem fileOpenItem = new MenuItem("Open");
        fileOpenItem.setAccelerator(KeyCombination.valueOf("Ctrl+O"));
        fileOpenItem.setOnAction(e -> {
            Alert alert = new Alert(Alert.AlertType.INFORMATION, "Ctrl+O pressed");
            alert.showAndWait();
        });

        Menu fileMenu = new Menu("File", null, fileOpenItem);
        return new MenuBar(fileMenu);
    }
}

Karl

@TomasMikula
Copy link
Member

The difference in your use cases might be in context menu vs. menu bar. Context menu probably gets the chance to handle the event first, before text area, and if it does not consume the event, text area's handler is still invoked. This is in contrast to menu bar, where the text area gets to handle the event first, and if it is not consumed, menu bar accelerator is triggered.

@JFormDesigner If RichTextFX seems to consume events it does not actually care about (like Ctrl+O), feel free to open a separate issue for that.

@JFormDesigner
Copy link
Contributor

Thanks, opened #185

@TomasMikula
Copy link
Member

@djskrien What JDK version are you using? I just experienced a similar problem (handling accelerator twice) with JDK 8u40 that goes away with JDK 8u60.

@djskrien
Copy link
Author

I'm using JDK 8u60 on a Mac. I haven't tried it on a PC.

@TomasMikula
Copy link
Member

OK, that is the same environment I have. Would be great if you could provide a self-contained example.

@djskrien
Copy link
Author

After a delay of almost four months, I have come up with a self-contained example of my problem, as you requested. I also know which line of code is causing the problem but I don't know how to fix it.

Here is some code that shows the problem:

import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.control.Menu;
import javafx.scene.control.MenuBar;
import javafx.scene.control.MenuItem;
import javafx.scene.input.KeyCombination;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;
import org.fxmisc.richtext.CodeArea;
import org.fxmisc.richtext.LineNumberFactory;
import org.fxmisc.richtext.StyleSpans;
import org.fxmisc.richtext.StyleSpansBuilder;

import java.util.Collection;
import java.util.Collections;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class JavaKeywords extends Application
{

    private static final String[] KEYWORDS = new String[]{"abstract", "assert",
            "boolean", "break", "byte", "case", "catch", "char", "class", "const",
            "continue", "default", "do", "double", "else", "enum", "extends", "final",
            "finally", "float", "for", "goto", "if", "implements", "import",
            "instanceof", "int", "interface", "long", "native", "new", "package",
            "private", "protected", "public", "return", "short", "static", "strictfp",
            "super", "switch", "synchronized", "this", "throw", "throws", "transient",
            "try", "void", "volatile", "while"};

    private static final String KEYWORD_PATTERN = "\\b(" + String.join("|", KEYWORDS) +
            ")\\b";
    private static final String PAREN_PATTERN = "\\(|\\)";
    private static final String BRACE_PATTERN = "\\{|\\}";
    private static final String BRACKET_PATTERN = "\\[|\\]";
    private static final String SEMICOLON_PATTERN = "\\;";
    private static final String STRING_PATTERN = "\"([^\"\\\\]|\\\\.)*\"";
    private static final String COMMENT_PATTERN = "//[^\n]*" + "|" + "/\\*(.|\\R)*?\\*/";

    private static final Pattern PATTERN = Pattern.compile("(?<KEYWORD>" +
            KEYWORD_PATTERN + ")" + "|(?<PAREN>" + PAREN_PATTERN + ")" + "|(?<BRACE>" +
            BRACE_PATTERN + ")" + "|(?<BRACKET>" + BRACKET_PATTERN + ")" + "|" +
            "(?<SEMICOLON>" + SEMICOLON_PATTERN + ")" + "|(?<STRING>" + STRING_PATTERN
            + ")" + "|(?<COMMENT>" + COMMENT_PATTERN + ")");

    private static final String sampleCode = String.join("\n", new String[]{"package "
            + "com.example;", "", "import java.util.*;", "", "public class Foo extends " +
            "" + "Bar implements Baz {", "", "    /*", "     * multi-line comment", "  " +
            "   " + "*/", "    public static void main(String[] args) {", "        // "
            + "single-line comment", "        for(String arg: args) {", "            " +
            "if" + "(arg.length() != 0)", "                System.out.println(arg);", "" +
            "       " + "     else", "                System.err.println(\"Warning: " +
            "empty string as" + " argument\");", "        }", "    }", "", "}"});


    public static void main(String[] args) {
        launch(args);
    }

    @Override
    public void start(Stage primaryStage) {
        CodeArea codeArea = new CodeArea();
        codeArea.setParagraphGraphicFactory(LineNumberFactory.get(codeArea));
        codeArea.richChanges().subscribe(change -> {
            codeArea.setStyleSpans(0, computeHighlighting(codeArea.getText()));
        });
        codeArea.replaceText(0, 0, sampleCode);

        MenuItem undoItem = new MenuItem("Undo");
        undoItem.setAccelerator(KeyCombination.valueOf("SHORTCUT+Z"));
        undoItem.setOnAction(e -> codeArea.undo());
        MenuBar menuBar = new MenuBar(new Menu("Edit", null, undoItem));
        menuBar.setUseSystemMenuBar(false);

        VBox root = new VBox(menuBar, codeArea);
        Scene scene = new Scene(root, 600, 400);
        scene.getStylesheets().add(JavaKeywords.class.getResource("java-keywords.css")
                .toExternalForm());

        primaryStage.setScene(scene);
        primaryStage.setTitle("Java Keywords");
        primaryStage.show();
    }

    private static StyleSpans<Collection<String>> computeHighlighting(String text) {
        Matcher matcher = PATTERN.matcher(text);
        int lastKwEnd = 0;
        StyleSpansBuilder<Collection<String>> spansBuilder = new StyleSpansBuilder<>();
        while (matcher.find()) {
            String styleClass = matcher.group("KEYWORD") != null ? "keyword" : matcher
                    .group("PAREN") != null ? "paren" : matcher.group("BRACE") != null
                    ? "brace" : matcher.group("BRACKET") != null ? "bracket" : matcher
                    .group("SEMICOLON") != null ? "semicolon" : matcher.group("STRING")
                    != null ? "string" : matcher.group("COMMENT") != null ? "comment" :
                    null; /* never happens */
            assert styleClass != null;
            spansBuilder.add(Collections.emptyList(), matcher.start() - lastKwEnd);
            spansBuilder.add(Collections.singleton(styleClass), matcher.end() - matcher
                    .start());
            lastKwEnd = matcher.end();
        }
        spansBuilder.add(Collections.emptyList(), text.length() - lastKwEnd);
        return spansBuilder.create();
    }
}

In the middle of the start( ) method, I added code to create a menubar, menu, and menu item. I then tried running this code on a Mac and it ran fine. So far so good.

But then I changed line 73 so that the parameter is true instead of false:

        menuBar.setUseSystemMenuBar(true);

After that change, every time I chose Cmd-Z to undo the last edit, it undid the last two edits, once because of the built-in undo action and once again because of the menu item's action.

Is this a problem with RichTextFX or is it a problem with JavaFX's implementation of setUseSystemMenuBar( ) for the Mac?

For reference, I'm using the latest version of RichTextFX (richtextfx-fat-0.6.10) and I'm using JDK 1.8.0_66 on a Mac running MacOS X 10.9.5.

@JordanMartinez
Copy link
Contributor

Regarding RichTextFX Undo...
Using just this code:

import javafx.application.Application;
import javafx.scene.Scene;
import javafx.stage.Stage;
import org.fxmisc.richtext.CodeArea;
import org.fxmisc.richtext.LineNumberFactory;

public class UndoTest extends Application {

    public static void main(String[] args) {
        launch(args);
    }

    @Override
    public void start(Stage primaryStage) {
        CodeArea codeArea = new CodeArea();
        codeArea.setParagraphGraphicFactory(LineNumberFactory.get(codeArea));

        String initialString = "Some sample of some code.\nAnother line for good measure.";
        codeArea.replaceText(0, 0, initialString);
        // Note: these lines are not called in this example nor djskrien's example:
        // codeArea.getUndoManager().forgetHistory();
        // codeArea.getUndoManager().mark();

        Scene scene = new Scene(codeArea, 400, 400);
        primaryStage.setScene(scene);
        primaryStage.show();
    }
}

I noticed that after the area is initialized with its text, after I type SPACE, SPACE, SPACE, K, SPACE, K, SPACE (" k k "), then pause for a second or two, and then type CTRL+Z, it puts the entire text back to nothing (undoes my text insertion and the initialString). So, it appeared that the area itself would "undo" twice: once for my text insertion, another for the actual text with which it was initialized.

So... Having said that, is that supposed to happen?

Regarding djskrien's example...
After I added a println statement to insure that menu item was working:

undoItem.setOnAction(ae -> {
    System.out.println("Menu Item calling undo()");
    codeArea.undo();
});

I found that the code in your menuItem in your example didn't even run because CodeArea still had the focus. Even if I clicked outside the stage or the menu item itself, the area still had the focus.

So, I tried it again by changing start() to include a ScrollPane that could receive the focus:

public void start(Stage primaryStage) {
    CodeArea codeArea = new CodeArea();
    codeArea.setParagraphGraphicFactory(LineNumberFactory.get(codeArea));
    codeArea.richChanges().subscribe(change -> {
        codeArea.setStyleSpans(0, computeHighlighting(codeArea.getText()));
    });
    codeArea.replaceText(0, 0, sampleCode);

    MenuItem undoItem = new MenuItem("Undo");
    undoItem.setAccelerator(KeyCombination.valueOf("SHORTCUT+Z"));
    undoItem.setOnAction(e -> {
        System.out.println("Menu Item: Undoing previous thing.");
        codeArea.undo();
    });
    MenuBar menuBar = new MenuBar(new Menu("Edit", null, undoItem));
    menuBar.setUseSystemMenuBar(true);

    VBox root = new VBox(
            menuBar, 
            codeArea, 
            // add a ScrollPane that can receive the focus to test if menuItem works.
            new ScrollPane(new Rectangle(50, 50)));

    Scene scene = new Scene(root, 600, 400);
    scene.getStylesheets().add(JavaKeywords.class.getResource("java-keywords.css")
            .toExternalForm());

    primaryStage.setScene(scene);
    primaryStage.setTitle("Java Keywords");
    primaryStage.show();
}

When I typed in some text, checked to insure that any undo() statement would only undo once (undo the text I inserted), and then clicked on the ScrollPane, typing CTRL+Z only called undo() once. It undid my text insertion and not sampleCode.

@JordanMartinez
Copy link
Contributor

I just remembered that your OS is for Mac. I tested this with Linux, so I'm not sure if my findings will be consistent with yours if you ran the same code.

Using...

java version "1.8.0_66"
Java(TM) SE Runtime Environment (build 1.8.0_66-b17)
Java HotSpot(TM) 64-Bit Server VM (build 25.66-b17, mixed mode)

@TomasMikula
Copy link
Member

Closing, as it seems to be the problem with the focus, as @JordanMartinez discovered. Please reopen if that doesn't solve the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants