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

Add a demo that shows how to override the default behavior and some… #292

Merged
merged 1 commit into from
Apr 27, 2016
Merged

Add a demo that shows how to override the default behavior and some… #292

merged 1 commit into from
Apr 27, 2016

Conversation

JordanMartinez
Copy link
Contributor

@JordanMartinez JordanMartinez commented Apr 19, 2016

… quirks of the new approach:

  • Shows how to override the default behavior of a node by installing an InputMap
  • Shows the bug (or feature?) that can arise when one adds a handler to the node's on[EventType]Property.

@JordanMartinez
Copy link
Contributor Author

I've updated this PR to be rebased on the master branch (and a few additional changes).

@JordanMartinez
Copy link
Contributor Author

@TomasMikula Is there an issue preventing you from merging this?

@TomasMikula
Copy link
Member

This demo is quite elaborate, demonstrating many things at once. I can see how 200 lines of code can be a little too much to process for someone who just wants to override a single key combination. Maybe sometimes less is more...

@TomasMikula
Copy link
Member

It shouldn't require internet connection to work.

@TomasMikula
Copy link
Member

Using a template only makes sense if it is going to be reused many times. For a one-off use, using InputMap directly is effectively the same, and probably easier.

@JordanMartinez
Copy link
Contributor Author

This demo is quite elaborate, demonstrating many things at once. I can see how 200 lines of code can be a little too much to process for someone who just wants to override a single key combination. Maybe sometimes less is more...

Good point. However, if someone does indeed subclass a StyledTextArea, doesn't the demonstration show the basic template one would follow to do that?

It shouldn't require internet connection to work.

I only included that to insure the link was valid. If it isn't and we don't catch it, then someone will file an issue and we could update it immediately. However, That is a good point.

Using a template only makes sense if it is going to be reused many times. For a one-off use, using InputMap directly is effectively the same, and probably easier.

Perhaps this demo should be broken up into 2-3 demos? One could show how to write a template for a subclass of StyledTextArea. The second could show how to override one part of the handler via an InputMap. Third (or part of the second one) would show what happens if one also adds an EventHandler to the area's on[EventType]Property.

@TomasMikula
Copy link
Member

I only included that to insure the link was valid. If it isn't and we don't catch it, then someone will file an issue and we could update it immediately. However, That is a good point.

We can just include the link in the README instead of the demo, and the same still applies (i.e. someone will file an issue).

A template can really only pay off when both

  1. it contains many event handlers (such as the one in StyledTextAreaBehavior), and
  2. there are many instances (e.g. if someone is implementing an IDE which will have many text areas).

When only a handful of event handlers is being defined/overridden, condition 1. does not hold. So I probably would not bother explaining templates in RichTextFX documentation.

Note that even when subclassing StyledTextArea, one can use InputMap directly; and even when not subclassing, one could use InputMapTemplate.

@JordanMartinez
Copy link
Contributor Author

We can just include the link in the README instead of the demo, and the same still applies (i.e. someone will file an issue).

Works for me. BTW, are you keeping track of all the work to be done on the ReadMe? I think the things we've discussed span 3-4 topics, but I could only recall one (explaining that STA is the base of the other flavors) when I thought about it.

So I probably would not bother explaining templates in RichTextFX documentation.

Ok, that makes sense. On another point, the API is the same for InputMap and InputMapTemplate, so most should be able to figure it out and, if not, it won't take long to explain.

@JordanMartinez
Copy link
Contributor Author

I've updated the PR.


EventHandler<KeyEvent> insertNewlineChar = e -> {
if (e.getCode().equals(ENTER)) {
area.insertText(area.getCaretPosition(), "\n");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You would want to call e.consume() here, to prevent the event from bubbling up the scene graph. The peculiarity is that other handlers on the same Node will still be invoked.
From Handling JavaFX Events:

However, if the node that consumes the event has more than one filter or handler registered for the event, the peer filters or handlers are still executed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. It's fixed now.

@TomasMikula
Copy link
Member

BTW, are you keeping track of all the work to be done on the ReadMe?

No, but I know someone is doing that here ;)

@JordanMartinez
Copy link
Contributor Author

No, but I know someone is doing that here ;)

Ah! I thought I opened an issue for that specifically, but I couldn't find it. So, I thought I had decided against doing that. 😀

…uirks of the new approach:

- Shows how to install an InputMap on a single instance of a node.
- Shows the bug (or feature?) that can arise when one adds a handler to the node's `on[EventType]Property`.
@TomasMikula
Copy link
Member

👍

@TomasMikula TomasMikula merged commit 10f3df4 into FXMisc:master Apr 27, 2016
@JordanMartinez JordanMartinez deleted the addOverrideBehaviorDemo branch April 27, 2016 01:26
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.

2 participants