Focus issue about jfxtras HOT 21 CLOSED

Well, reusing the CalendarTextField in the LocalDate*TextField has saved me a lot of duplicate code (and corresponding maintenance), but it comes at a price that you have such forwarding code.

I'm not sure why the runLater is there; apparently it was needed to make it work :-)

Would you mind creating a PR for this change, this time with a test?

from jfxtras.

Yes I'll do it as soon as possible!

from jfxtras.

I'm working on the PR and doing some tests on my application.

I came across this bug because I want to put the editor inside a TableCell when edition is requested on a TableView.
What I do is that I request the focus and do a "select all" inside the Textfield. Therefore, if the user is double clicking on a cell, he can start typing directly and the text is erased by what he's typing.

Also if the user is typing directly into a cell not in edition, I manually start the edition. But because the runLater is there, the focus is not given immediately, and the user's input is gone. Can I remove this runLater in the code? There is no comment explaining why it was there in the first place..

Also, do you have any idea how to make public the "selectAll" method of the TextField through the LocalDateTextField?

from jfxtras.

Apparently the runLater was needed, otherwise it would not be there. A comment would have been good, shame on me. That said, the original code was pretty darn old, back to the prerelease of JavaFX 2.0, so chances are focus handling has changed. You may remove the runLater if all the test keep running.

The selectAll, because the control is named "TextField" it is allowed to assume that its skin will implement a TextField, so the selectAll method can be added to the Calendar/LocalDateTextField control and forwarded to the skin. For this I usually create an interface and have the control assume all skins have implemented that.

from jfxtras.

Allright, I'll run the tests but I don't think the runLater is needed.

How would you forward the selectAll()? You mean creating an Interface for the skin containing like :
public void selectAll();

And the Calendar/LocalDateTextField will have :

public void selectAll(){
((MySkin)getSkin()).selectAll();
}

Something like that..?

from jfxtras.

Exactly like that. The interface requires what the skins must implement. But this is only applicable for the CalendarTextField skin(s), the LocalDateTextField will forward the selectAll at control level (just like the focus change).

from jfxtras.

But the focus change is forwarded in the LocalDateTextFieldSkin .

So the selectAll method in the LocalDateTextField will call the selectAll of the LocalDateTextFieldSkin, which will call selectAll of the CalendarTextField control?

from jfxtras.

Yes, CalendarTextField will have a selectAll(). LocalDateTextFieldSkin uses a CalendarTextField control without addressing its skin directly, so it should call the selectAll() on CalendarTextField.

Please also add the selectAll on the other *TextField controls?

from jfxtras.

Allright I think I got it.
Yes I will add this method on other controls and some tests as soon as possible.

from jfxtras.

Well without modifying any files, I have some test failing..
Is the test "slideStep15" of CalendarTimePickerTest of JfXtras Controls succeeding on your side?

from jfxtras.

Do you use a mac?

from jfxtras.

Nope I have a Windows..

Le 21 nov. 2016 20:30, "Tom Eugelink" [email protected] a écrit :

Do you use a mac?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#73 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AIi-baN5duC-HAYINTCxgl_seRtotOVJks5rAfFegaJpZM4Ku8o_
.

from jfxtras.

That is strange (so have I).
Yes, last time I ran the tests, a week ago or so, all ran ok.

from jfxtras.

I see it failing now as well.

from jfxtras.

My bad! Fixed it.

from jfxtras.

Allright, I've commited the things on my repo. Now I have (again!) another issue !

When the popup is shown, and you press escape, the focus is sent to the next element. That annoys me and it doesn't feel right. Maybe you clicked on the calendar (or maybe it was opened by the code) but you actually want to do some input with the keyboard. Thus you press escape, and you want to type something.

But since the focus is away, you have to manually click inside the TextField to input something. I think putting the focus inside the TextField after the popup is hidden is better for keyboard user experience.

Any thought on that?

The test is ready (all other tests are running with the modification )and should be looking like that :

@Test
    public void inputAfterEscape() {
        // open the popup
        TestUtil.runThenWaitForPaintPulse(() -> {
            calendarTextField.setPickerShowing(true);
        });
        type(KeyCode.ESCAPE);

        type(KeyCode.NUMPAD2);

        //TextField should be focused
        Assert.assertTrue(find(".text-field").isFocused());

        Assert.assertEquals("2", ((TextField) find(".text-field")).getText());
    }

I have to look a bit deeper because the CalendarTimeTextFieldSkin doesn't have a setOnHiding method like the CalendarTextFieldSkin, but I think it should also have.

from jfxtras.

Apparently you are the first one to put the control through its paces like this. Thank you for that. I'll get back to you.

from jfxtras.

Also on a side note, what is "getText()" supposed to give in CalendarTextField? Should it return the exact same thing as "getText()" on the TextField?

Because apparently, it's updated only when a value is set, or when we parse the Text. But we don't parse it each time a key is typed. So since the documentation is a bit light, I'm wondering if this "textProperty" should reflect the "textProperty" of the Textfield, or it has another purpose that I'm missing.

EDIT : I'm sorry to ask so many questions, but I'm using these calendars in very specific mode where everything is important.. I don't want to fork the project and do things on my side. I really want to give my feedback on real applications in order to improve these controls for other people. And like always, I'm able to provide pull requests for every demand I make.

from jfxtras.

You're question are highly appreciated! Yes, getText should return what getText on textfield does.

from jfxtras.

Allright, I'll try to modify the management of this textProperty and add some tests.

from jfxtras.

I've opened a separate issue for the getText issue. Waiting on your opinion regarding the focus on popup hiding. When you have some spare time of course ;)

from jfxtras.