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

fixed -fx-highlight-fill and -fx-caret-blink-rate (issue #303) #398

Merged
merged 2 commits into from
Jan 17, 2017

Conversation

JFormDesigner
Copy link
Contributor

No description provided.

@JordanMartinez
Copy link
Contributor

What is this supposed to do?

@alt-grr
Copy link

alt-grr commented Nov 17, 2016

@JordanMartinez Fixing issue #303 ?

@JFormDesigner
Copy link
Contributor Author

@kuc yes, this fixes #303

I'm also working on a fix for white selection text color (-fx-highlight-text-fill). Coming soon...

@JordanMartinez
Copy link
Contributor

@JFormDesigner & @kuc Thanks for the clarification. At the same time, how does this fix that issue? Because my original reply to that issue showed that it was a coding issue and I don't see how creating a list of CSS Metadata helps. (I'm not as familiar with CSS as I should be.)

@JFormDesigner
Copy link
Contributor Author

@JordanMartinez getCssMetaData() is invoked to ask a node what CSS properties it supports. It is the "connection" between CSS and node properties. IOW without getCssMetaData(), CSS attribute -fx-highlight-fill is simply ignored, because JavaFX does not know that StyledTextArea does support -fx-highlight-fill.

@JFormDesigner
Copy link
Contributor Author

BTW this PR does not fix -fx-highlight-text-fill (text color of selection),
only -fx-highlight-fill (background color of selection).

@JordanMartinez
Copy link
Contributor

@JFormDesigner Ah! Ok. I was confused because I felt like the code you implemented would only do something if StyledTextArea extended Control. That just shows a misunderstanding on my part about getCssMetaData() and how it works.

@JFormDesigner
Copy link
Contributor Author

updated (and tested) to current master branch

@JordanMartinez
Copy link
Contributor

JordanMartinez commented Jan 17, 2017

@JFormDesigner Could you post the code you used to test this? I checked on my side and the background color of the selection did not change to red. If I am doing something wrong, let me know. Otherwise, it seems the changes you made, while still needed, don't fix the whole problem.

After updating styled-text-area.css to:

.styled-text-area .virtual-flow {
    -fx-cursor: text;
    -fx-highlight-fill: red;
    -fx-blink-caret-rate: 100ms;
    -fx-font-size: 32pt;
}

and fixing the getCssMetaData method to what you have in this PR,
and using this code...

import javafx.application.Application;
import javafx.scene.Scene;
import javafx.stage.Stage;
import org.fxmisc.flowless.VirtualizedScrollPane;
import org.fxmisc.richtext.StyleClassedTextArea;
import org.fxmisc.richtext.StyledTextArea;

import java.util.Collection;
import java.util.Collections;

public class CssTest extends Application {

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

    @Override
    public void start(Stage primaryStage) {
        StyledTextArea<String, Collection<String>> area = new StyledTextArea<String, Collection<String>>("", (a, b) -> {},
                Collections.emptyList(), (a, b) -> a.getStyleClass().addAll(b));
        area.replaceText("Some text in the area");
        VirtualizedScrollPane<StyledTextArea<String, Collection<String>>> vsPane = new VirtualizedScrollPane<>(area);

        Scene scene = new Scene(vsPane, 500, 500);
        primaryStage.setScene(scene);
        primaryStage.show();
    }
}

System:

Linux Mint 18.1

java version "1.8.0_111"
Java(TM) SE Runtime Environment (build 1.8.0_111-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.111-b14, mixed mode)

@JFormDesigner
Copy link
Contributor Author

@JordanMartinez it works without .virtual-flow in the CSS selector:

.styled-text-area {
    -fx-highlight-fill: red;
    -fx-caret-blink-rate: 100ms;
}

BTW I've rebased the PR to current master; this PR also fixes #426

@JFormDesigner
Copy link
Contributor Author

In Markdown Writer FX I use a brighter color, which makes selected text better readable but still identifiable as selection:

.styled-text-area {
    -fx-highlight-fill: derive(#1E90FF, 50%);
}

Comparison:
image

Maybe you could consider adding this to styled-text-area.css?

The optimal solution would be changing selected text color (-fx-highlight-text-fill) to white, but this does not work because of JDK bug https://bugs.openjdk.java.net/browse/JDK-8149134. See PR #428

@JordanMartinez
Copy link
Contributor

Ah, ok! Could you also add a commit that updates the styled-text-area.css file to remove the .virtual-flow from it?

I'm not sure if there would need to be another area later in that file that writes something like...

.virtual-flow {
    -fx-cursor: text;
}

@JFormDesigner
Copy link
Contributor Author

Could you also add a commit that updates the styled-text-area.css file to remove the .virtual-flow from it?

Sure. Done.

@JordanMartinez JordanMartinez merged commit a1df657 into FXMisc:master Jan 17, 2017
@JordanMartinez
Copy link
Contributor

Great! Thanks for your work and patience.

@JFormDesigner JFormDesigner deleted the fx-highlight-fill-fix branch January 18, 2017 08:02
@JordanMartinez
Copy link
Contributor

@JFormDesigner I didn't say beforehand, but I like the default color you use when selecting a text as you mentioned in your above comment.

What if you submitted a PR that updates that in the css file and then update #428 to revert that fix, so that when the JDK bug is fixed, it'll be back to what it was before?

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

Successfully merging this pull request may close these issues.

4 participants