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

The positionCaret does not work #293

Closed
WinXaito opened this issue Apr 21, 2016 · 20 comments
Closed

The positionCaret does not work #293

WinXaito opened this issue Apr 21, 2016 · 20 comments

Comments

@WinXaito
Copy link

Hello,

I try to move my cursor in my StyleClassedTextArea with the functionSourceText.positionCaret(0);, I see the cursor move, but when I write, it resumes where it was before the shifting.

How can I fix this?

Thanking you

(Translated with Google translation)

Original:
Bonjour,

j'essaye de déplacer mon curseur dans mon StyleClassedTextArea, avec la function SourceText.positionCaret(0);, je vois bien le curseur se déplacer, mais dès que j'écris, il reprend la où il était avant le déplacement.

Comment puis-je régler ce problème ?

En vous remerciant

@TomasMikula
Copy link
Member

Hi,

can you please provide a simple self-contained example (single Java file) that demonstrates the problem? Also, what RichTextFX version are you using?

@WinXaito
Copy link
Author

Yes,

The project (Zest-Writer) is Open-Source.

Here is a commit with the problem: https://github.com/WinXaito/zest-writer/blob/b9b34479aa5a15481bbb7060831b305582db593a/src/main/java/com/zestedesavoir/zestwriter/view/MdConvertController.java#L106

(When I click on the "Bold" button, I gives focus to the editor and I would like to position the cursor before 2 position (Here I put 0 for example).

Thanks !

@JordanMartinez
Copy link
Contributor

He's using the 0.6.10 release.

@WinXaito
Copy link
Author

And it's a problem ?

I see that this is the last release https://github.com/TomasMikula/RichTextFX/releases

@TomasMikula
Copy link
Member

TomasMikula commented Apr 21, 2016

A self-contained example is one that can be run without any dependencies. A simple example is one that is minimal such that it still demonstrates the issue. See SSCCE. This really is to increase your chance to get a helpful answer more quickly.

@JordanMartinez
Copy link
Contributor

I compiled their code. The issue is that they don't call StyledTextArea#requestFocus.

In their code, when the user clicks a button (Bold, Italic), they want to style the selected text. However, upon clicking that button, the area no longer has the focus. Since the 0.6.10 release hard-codes the caret blink (only blinks when caret has focus, among other things), visibly, the caret isn't updated. So, even they they are indeed updating the caret, it does not appear to do so visibly because the area no longer has the focus.

@TomasMikula
Copy link
Member

Thanks for investigating. That doesn't really correspond with this description, though:

I see the cursor move, but when I write, it resumes where it was before the shifting.

@WinXaito does calling area.requestFocus() solve the problem?

@JordanMartinez
Copy link
Contributor

Turns out I had the wrong commit checked out. I'm looking into the one they've referenced.

@JordanMartinez
Copy link
Contributor

JordanMartinez commented Apr 21, 2016

Ah... I see what's going on now.

Let's say the area has the following text:

Some stuff here

I select "here" and click the bold button. The caret is repositioned at 0. Now the text and caret appear as below:

|Some stuff **here**

As soon as I start typing anything (the area at this point has the focus), the caret will jump to the end of "here". Let's say I wrote "duck", the area would appear as follows:

Some stuff **here**duck|

@TomasMikula
Copy link
Member

When you start typing again, is "here" still selected? That would explain it, and using moveTo instead of positionCaret should solve it.

@JordanMartinez
Copy link
Contributor

"here" is not still selected. moveTo fixes the issue. I'll let them know.

@TomasMikula
Copy link
Member

I think positionCaret was made public only because StyledTextAreaBehavior needs it, and StyledTextAreaBehavior used to be in a different package than StyledTextArea. Javadoc says "Use with care."

@JordanMartinez
Copy link
Contributor

We might want to look into that. There might be other methods that are public but which no longer need to be.

Also, I've been curious about something else: why are some of the area's methods "hidden" in interfaces? For example, moveTo isn't actually listed in STA's source code. Rather, it's inside the NavigationActions interface. This approach would make sense to me if there were other classes implementing that same interface, but there aren't any others.

So, is this simply a way to group related methods together to make it easier to find them in the javadoc?

@TomasMikula
Copy link
Member

There are some "core" methods, and then there are "derived" methods that can be implemented on top of the core methods. I wanted to implement the derived methods "somewhere else", so that StyledTextArea, which is pretty large anyway, can focus only on the core methods. I was hoping that it would make StyledTextArea's code cleaner. I admit it is somewhat arguable whether it really does.

I'm not sure it helps Javadoc.

Regardless, it may become handy to have an interface with all the editing methods that is not a subclass of Node (StyledTextAreaModel kind of is that, except it is not an interface).

@TomasMikula
Copy link
Member

@WinXaito Can you confirm that using moveTo solves your problem?

@WinXaito
Copy link
Author

I was testing right now, I'll let you know!

@WinXaito
Copy link
Author

Yes, it works perfectly! Thank you very much.

@JordanMartinez
Copy link
Contributor

Regardless, it may become handy to have an interface with all the editing methods that is not a subclass of Node (StyledTextAreaModel kind of is that, except it is not an interface).

The strengths of the current approach is that all methods relating to the same topic (say navigation) are in the same file/interface. However, it also makes it somewhat tedious to follow the trail of all these methods in the source code (I don't think it's as bad for the javadoc...).
Let's say we do refactor all the editing interfaces into one interface. This removes that method-trail, but then the natural grouping of the methods is no longer there. So how do we keep that? One way would be to add javadoc to that new single interface that groups methods together by topic and then links to them. For example:

/**
<h1>Navigation Methods</h1>
<ul>
   <li>{@link #moveTo(int position)}</li>
   <li>{@link #someOtherNavMethod(Object args)}</li>
</ul>

However, this breaks the principle of self-documenting code, which the current approach seems to uphold better than this new approach.

On another topic, I must admit that I've been using positionCaret this entire time in my code, albeit without any issues. I never looked at the javadoc and saw that it might cause a problem (probably because I never had one). We might want to add a note to both methods' javadocs as well informing users that moveTo is the primary method to use with positionCaret being the alternative to use in rare/unusual cases.

@TomasMikula
Copy link
Member

Let's deprecate positionCaret, with explaining that they probably wanted to use moveTo anyway and that it will be made package-private in a future version.

I would refrain from reorganizing the interfaces for now, until it is clear to us what a better organization would be.

@JordanMartinez
Copy link
Contributor

Let's deprecate positionCaret, with explaining that they probably wanted to use moveTo anyway and that it will be made package-private in a future version.

I would refrain from reorganizing the interfaces for now, until it is clear to us what a better organization would be.

Both ideas sound good to me.

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

3 participants