dlsc-software-consulting-gmbh / GemsFX

A collection of JavaFX controls and utilities.
Apache License 2.0
453 stars 54 forks source link

Time picker bindings #68

Closed cpreisler closed 1 year ago

cpreisler commented 1 year ago

Added secondFieldVisibleProperty and millisecondFieldVisibleProperty to TimePicker and changed TimePickerSkin to bind the managed and visible properties to these new properties. This allows users to change and see the format of the TimePicker in SceneBuilder. I also slightly changed the handling of NEW_TIME property change and handling of set minimum to handle setting the TimePicker time to null multiple times and then editing the field. Previously the time was not always set and the NEW_TIME property was not handled correctly and the minimum value for a field was set when the time was null.

cpreisler commented 1 year ago

I agree, this is a little strange. The issue I ran into was in SceneBuilder. Changing the "format" property did not change the view in SceneBuilder or in the preview in SceneBuilder. The only way I could get this to work in SceneBuilder was to introduce the properties and bind them directly to the controls in the skin. It's not ideal either. In SceneBuilder the user needs to set all three properties to show milliseconds as the popup does not work correctly if the format is not set. So my FXML looks like this in the end.

<TimePicker fx:id="startTimeField" disable="true" format="HOURS_MINUTES_SECONDS_MILLIS" millisecondFieldVisible="true" secondFieldVisible="true">

I did not change the demo application. If a developer is just writing code, then all they need to do is set the format property. I briefly did some research on why setting the enum based property in SceneBuilder was not working, but didn't find an answer. I think it's something to do with how SceneBuilder instantiates a new control when a property changes.

If you don't want these changes, I totally understand. If you have an idea for a different solution, I can try to make that work too. I like using SceneBuilder, so having it work correctly there is important to me.

On Sun, Apr 2, 2023 at 10:41 AM Dirk Lemmermann @.***> wrote:

@.**** requested changes on this pull request.

Why do we need boolean properties now for showing the seconds and millisecond fields? We do have the "format" property for exactly that. Doesn't your change mean I could show milliseconds without seconds? I don't understand.

In gemsfx-demo/nbactions.xml https://github.com/dlsc-software-consulting-gmbh/GemsFX/pull/68#discussion_r1155333186 :

@@ -0,0 +1,67 @@ +<?xml version="1.0" encoding="UTF-8"?>

Please delete netbeans config file from project.

— Reply to this email directly, view it on GitHub https://github.com/dlsc-software-consulting-gmbh/GemsFX/pull/68#pullrequestreview-1368104487, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADVDHDR67CPWZKB3SDR4OOLW7GM4NANCNFSM6AAAAAAWQKILFU . You are receiving this because you authored the thread.Message ID: @.*** com>

dlemmermann commented 1 year ago

You should create a ticket for the SceneBuilder project then.

cpreisler @.***> schrieb am So. 2. Apr. 2023 um 18:12:

I agree, this is a little strange. The issue I ran into was in SceneBuilder. Changing the "format" property did not change the view in SceneBuilder or in the preview in SceneBuilder. The only way I could get this to work in SceneBuilder was to introduce the properties and bind them directly to the controls in the skin. It's not ideal either. In SceneBuilder the user needs to set all three properties to show milliseconds as the popup does not work correctly if the format is not set. So my FXML looks like this in the end.

<TimePicker fx:id="startTimeField" disable="true" format="HOURS_MINUTES_SECONDS_MILLIS" millisecondFieldVisible="true" secondFieldVisible="true">

I did not change the demo application. If a developer is just writing code, then all they need to do is set the format property. I briefly did some research on why setting the enum based property in SceneBuilder was not working, but didn't find an answer. I think it's something to do with how SceneBuilder instantiates a new control when a property changes.

If you don't want these changes, I totally understand. If you have an idea for a different solution, I can try to make that work too. I like using SceneBuilder, so having it work correctly there is important to me.

On Sun, Apr 2, 2023 at 10:41 AM Dirk Lemmermann @.***> wrote:

@.**** requested changes on this pull request.

Why do we need boolean properties now for showing the seconds and millisecond fields? We do have the "format" property for exactly that. Doesn't your change mean I could show milliseconds without seconds? I don't understand.

In gemsfx-demo/nbactions.xml < https://github.com/dlsc-software-consulting-gmbh/GemsFX/pull/68#discussion_r1155333186

:

@@ -0,0 +1,67 @@ +<?xml version="1.0" encoding="UTF-8"?>

Please delete netbeans config file from project.

— Reply to this email directly, view it on GitHub < https://github.com/dlsc-software-consulting-gmbh/GemsFX/pull/68#pullrequestreview-1368104487 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/ADVDHDR67CPWZKB3SDR4OOLW7GM4NANCNFSM6AAAAAAWQKILFU

. You are receiving this because you authored the thread.Message ID: @.*** com>

— Reply to this email directly, view it on GitHub https://github.com/dlsc-software-consulting-gmbh/GemsFX/pull/68#issuecomment-1493381744, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACIXWXOVX7RU37JM2IELUVDW7GQNXANCNFSM6AAAAAAWQKILFU . You are receiving this because you commented.Message ID: @.***>

-- Dirk Lemmermann

Software & Consulting Zurich, Switzerland +41-(0)79-800-23-20 http://www.dlsc.com @.***

cpreisler commented 1 year ago

I will test it a little more. It appears that setting the FXML property with the current 1.71.0 build does not work either. The second/millisecond fields do not appear when running the application. In my FMXL I have this.

<TimePicker fx:id="startTimeField" disable="true" format="HOURS_MINUTES_SECONDS_MILLIS" showPopupTriggerButton="false">

When running the application the seconds and millis do not appear. The popup button is properly hidden.

On Sun, Apr 2, 2023 at 11:17 AM Dirk Lemmermann @.***> wrote:

You should create a ticket for the SceneBuilder project then.

cpreisler @.***> schrieb am So. 2. Apr. 2023 um 18:12:

I agree, this is a little strange. The issue I ran into was in SceneBuilder. Changing the "format" property did not change the view in SceneBuilder or in the preview in SceneBuilder. The only way I could get this to work in SceneBuilder was to introduce the properties and bind them directly to the controls in the skin. It's not ideal either. In SceneBuilder the user needs to set all three properties to show milliseconds as the popup does not work correctly if the format is not set. So my FXML looks like this in the end.

<TimePicker fx:id="startTimeField" disable="true" format="HOURS_MINUTES_SECONDS_MILLIS" millisecondFieldVisible="true" secondFieldVisible="true">

I did not change the demo application. If a developer is just writing code, then all they need to do is set the format property. I briefly did some research on why setting the enum based property in SceneBuilder was not working, but didn't find an answer. I think it's something to do with how SceneBuilder instantiates a new control when a property changes.

If you don't want these changes, I totally understand. If you have an idea for a different solution, I can try to make that work too. I like using SceneBuilder, so having it work correctly there is important to me.

On Sun, Apr 2, 2023 at 10:41 AM Dirk Lemmermann @.***> wrote:

@.**** requested changes on this pull request.

Why do we need boolean properties now for showing the seconds and millisecond fields? We do have the "format" property for exactly that. Doesn't your change mean I could show milliseconds without seconds? I don't understand.

In gemsfx-demo/nbactions.xml <

https://github.com/dlsc-software-consulting-gmbh/GemsFX/pull/68#discussion_r1155333186

:

@@ -0,0 +1,67 @@ +<?xml version="1.0" encoding="UTF-8"?>

Please delete netbeans config file from project.

— Reply to this email directly, view it on GitHub <

https://github.com/dlsc-software-consulting-gmbh/GemsFX/pull/68#pullrequestreview-1368104487

, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/ADVDHDR67CPWZKB3SDR4OOLW7GM4NANCNFSM6AAAAAAWQKILFU

. You are receiving this because you authored the thread.Message ID: @.*** com>

— Reply to this email directly, view it on GitHub < https://github.com/dlsc-software-consulting-gmbh/GemsFX/pull/68#issuecomment-1493381744 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/ACIXWXOVX7RU37JM2IELUVDW7GQNXANCNFSM6AAAAAAWQKILFU

. You are receiving this because you commented.Message ID: @.***>

-- Dirk Lemmermann

Software & Consulting Zurich, Switzerland +41-(0)79-800-23-20 http://www.dlsc.com @.***

— Reply to this email directly, view it on GitHub https://github.com/dlsc-software-consulting-gmbh/GemsFX/pull/68#issuecomment-1493382855, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADVDHDSQSPAOZXIC6OPEOGDW7GRBXANCNFSM6AAAAAAWQKILFU . You are receiving this because you authored the thread.Message ID: @.***>

cpreisler commented 1 year ago

I removed the second/millisecond properties and fixed the code so that setting the format from FXML (and consequently SceneBuilder) works.