Re: Proof of concept for fluent bindings for ObservableValue

2021-04-06 Thread Nir Lisker
> > In the PoC I made I specifically also disallowed 'null' as an input > I like the way ReactFX does it where the property is empty. I think that this is also what you mean by disallowing `null` (in other contexts, "disallowing null" would mean throwing an exception). Not entirely sure what you

Re: RFR: 8217955: Problems with touch input and JavaFX 11

2021-04-06 Thread Thiago Milczarek Sayao
On Mon, 5 Apr 2021 22:25:55 GMT, Kevin Rushforth wrote: >> This PR filters out `GDK_TOUCH_EVENT_MASK` from `GDK_ALL_EVENTS_MASK` to >> prevent touch events from being used instead of regular mouse events on >> Linux platforms. Note that the touch events will be delivered as mouse >>

Re: RFR: 8263807: Button types of a DialogPane are set twice, returns a wrong button [v2]

2021-04-06 Thread Kevin Rushforth
On Tue, 6 Apr 2021 16:01:15 GMT, Kevin Rushforth wrote: >> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 8263807: Code review changes > > Marked as reviewed by kcr (Lead). > Interesting enough I don't need to enable

Re: RFR: 8263807: Button types of a DialogPane are set twice, returns a wrong button [v2]

2021-04-06 Thread Kevin Rushforth
On Tue, 6 Apr 2021 15:43:43 GMT, Marius Hanl wrote: >> When DialogPane#getButtonTypes().setAll() is called twice with the same >> argument(s), DialogPane#lookupButton does not return the node which is shown >> inside the button bar. >> This is due DialogPane adding two list change listeners

Re: RFR: 8263807: Button types of a DialogPane are set twice, returns a wrong button [v2]

2021-04-06 Thread Marius Hanl
On Tue, 6 Apr 2021 12:18:49 GMT, Kevin Rushforth wrote: >> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 8263807: Code review changes > > The fix and test look fine. There is one needed change as noted below. > > >

Re: RFR: 8263807: Button types of a DialogPane are set twice, returns a wrong button [v2]

2021-04-06 Thread Marius Hanl
On Tue, 6 Apr 2021 12:06:05 GMT, Kevin Rushforth wrote: >> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 8263807: Code review changes > > modules/javafx.controls/src/test/java/test/javafx/scene/control/DialogPaneTest.java

Re: RFR: 8263807: Button types of a DialogPane are set twice, returns a wrong button [v2]

2021-04-06 Thread Marius Hanl
> When DialogPane#getButtonTypes().setAll() is called twice with the same > argument(s), DialogPane#lookupButton does not return the node which is shown > inside the button bar. > This is due DialogPane adding two list change listeners to 'buttons' > (#getButtonTypes). They have the wrong

Re: RFR: 8264770: BidirectionalBinding should use InvalidationListener to prevent boxing

2021-04-06 Thread mstr2
On Tue, 6 Apr 2021 14:29:28 GMT, Nir Lisker wrote: > The benchmark might not tell the real story. To test these sorts of > performance changes you have to use JMH. There's too much relating to JIT > optimizations and JVM startup to be able to rely on the current benchmark. > The theoretical

Re: RFR: 8258777: SkinBase: add api to un-/register invalidation-/listChange listeners [v5]

2021-04-06 Thread Kevin Rushforth
On Tue, 6 Apr 2021 14:16:03 GMT, Jeanette Winzenburg wrote: >> I guess I didn't look closely enough. Mystery solved, though. > >> I haven't gone back and done a detailed review yet, but I like the overall >> changes. The one thing I did notice is that the language used to describe >> the

Re: RFR: 8264770: BidirectionalBinding should use InvalidationListener to prevent boxing

2021-04-06 Thread Nir Lisker
On Tue, 6 Apr 2021 12:08:39 GMT, Kevin Rushforth wrote: >>> Seems like I forgot to hit the send button on the webform. Here's the >>> tracking number: 9069787. >> >> I see it now. And thanks for providing the benchmark. That's what I was >> looking for. > > The bug is now visible here:

Re: RFR: 8258777: SkinBase: add api to un-/register invalidation-/listChange listeners [v5]

2021-04-06 Thread Jeanette Winzenburg
On Thu, 1 Apr 2021 15:14:15 GMT, Kevin Rushforth wrote: >> ahh .. looks like you marked the conversation that contained the original >> comment as resolved: >> https://github.com/openjdk/jfx/pull/409#discussion_r604387696 .. > > I guess I didn't look closely enough. Mystery solved, though. >

Re: RFR: 8258777: SkinBase: add api to un-/register invalidation-/listChange listeners [v6]

2021-04-06 Thread Jeanette Winzenburg
> Changes in Lambda..Handler: > - added api and implemenation to support invalidation and listChange > listeners in the same way as changeListeners > - added java doc > - added tests > > Changes in SkinBase > - added api (and implementation delegating to the handler) > - copied java doc from

Re: RFR: 8258777: SkinBase: add api to un-/register invalidation-/listChange listeners [v5]

2021-04-06 Thread Kevin Rushforth
On Thu, 1 Apr 2021 15:10:45 GMT, Jeanette Winzenburg wrote: >> Maybe GitHub decided to prune it.  >> >> Btw, you tagged the wrong "Kevin" above. > > ahh .. looks like you marked the conversation that contained the original > comment as resolved: >

Re: RFR: 8258777: SkinBase: add api to un-/register invalidation-/listChange listeners [v5]

2021-04-06 Thread Jeanette Winzenburg
On Thu, 1 Apr 2021 15:09:34 GMT, Kevin Rushforth wrote: >> @Kevin >> >> don't worry, I have seen your comment (and been thinking about it :) - but >> also can't find it right now (nor can I find Nir's comment which started the >> overhaul ..), no idea what happened. >> >> Seeing your

Re: RFR: 8264770: BidirectionalBinding should use InvalidationListener to prevent boxing

2021-04-06 Thread mstr2
On Mon, 5 Apr 2021 21:54:40 GMT, Kevin Rushforth wrote: >> The internal BidirectionalBinding class implements bidirectional bindings >> for JavaFX properties. The design intent of this class is to provide >> specializations for primitive value types to prevent boxing conversions (cf. >>

Re: RFR: 8264770: BidirectionalBinding should use InvalidationListener to prevent boxing

2021-04-06 Thread Kevin Rushforth
On Mon, 5 Apr 2021 23:38:29 GMT, Kevin Rushforth wrote: >> Seems like I forgot to hit the send button on the webform. Here's the >> tracking number: 9069787. >> >> I've used the following manual benchmark, which bidirectionally binds two >> properties and then produces a billion change

Re: RFR: 8264770: BidirectionalBinding should use InvalidationListener to prevent boxing

2021-04-06 Thread Kevin Rushforth
On Mon, 5 Apr 2021 23:16:53 GMT, mstr2 wrote: > Seems like I forgot to hit the send button on the webform. Here's the > tracking number: 9069787. I see it now. And thanks for providing the benchmark. That's what I was looking for. - PR: https://git.openjdk.java.net/jfx/pull/454

RFR: 8264770: BidirectionalBinding should use InvalidationListener to prevent boxing

2021-04-06 Thread mstr2
The internal BidirectionalBinding class implements bidirectional bindings for JavaFX properties. The design intent of this class is to provide specializations for primitive value types to prevent boxing conversions (cf. specializations of the Property class with a similar design intent).

Re: RFR: 8264770: BidirectionalBinding should use InvalidationListener to prevent boxing

2021-04-06 Thread Kevin Rushforth
On Sat, 3 Apr 2021 15:20:41 GMT, mstr2 wrote: > The internal BidirectionalBinding class implements bidirectional bindings for > JavaFX properties. The design intent of this class is to provide > specializations for primitive value types to prevent boxing conversions (cf. > specializations of

Re: RFR: 8264127: ListCell editing status is true, when index changes while editing

2021-04-06 Thread Jeanette Winzenburg
On Sat, 3 Apr 2021 15:20:33 GMT, Florian Kirmaier wrote: > @kleopatra > I've added another test for the case, when index changes in such a way, that > the cell should no longer be in the editing state, > and the test seems to work. Do I miss something? hmm .. that's weird: your test (cell 1

Re: RFR: 8263322: Calling Application.launch on FX thread should throw IllegalStateException, but causes deadlock [v9]

2021-04-06 Thread Florian Kirmaier
On Tue, 6 Apr 2021 12:23:28 GMT, Kevin Rushforth wrote: >> I've clicked now the finalize button. > > @FlorianKirmaier The CSR needs to be updated (fixVersion set to `openjfx17`) > and moved to Finalize again. The fixVersion is now openfjx17 and it's moved to "Finalized" - PR:

Re: RFR: 8263322: Calling Application.launch on FX thread should throw IllegalStateException, but causes deadlock [v9]

2021-04-06 Thread Kevin Rushforth
On Wed, 31 Mar 2021 13:53:35 GMT, Florian Kirmaier wrote: >> I reviewed the CSR, and it is now ready for you to Finalize. > > I've clicked now the finalize button. @FlorianKirmaier The CSR needs to be updated (fixVersion set to `openjfx17`) and moved to Finalize again. - PR:

Re: RFR: 8264677 MemoryLeak: Progressindicator leaks, when treeShowing is false [v2]

2021-04-06 Thread Florian Kirmaier
> Fixing leak in ProgressIndicator when treeShowing is false Florian Kirmaier has updated the pull request incrementally with one additional commit since the last revision: 8264677 changes based on code review - Changes: - all: https://git.openjdk.java.net/jfx/pull/455/files

Re: RFR: 8264677 MemoryLeak: Progressindicator leaks, when treeShowing is false [v2]

2021-04-06 Thread Florian Kirmaier
On Mon, 5 Apr 2021 22:47:54 GMT, Kevin Rushforth wrote: >> Florian Kirmaier has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8264677 >> changes based on code review > > I haven't run it yet, but noticed a couple things during a quick

Re: RFR: 8263807: Button types of a DialogPane are set twice, returns a wrong button

2021-04-06 Thread Kevin Rushforth
On Thu, 18 Mar 2021 14:38:18 GMT, Marius Hanl wrote: > When DialogPane#getButtonTypes().setAll() is called twice with the same > argument(s), DialogPane#lookupButton does not return the node which is shown > inside the button bar. > This is due DialogPane adding two list change listeners to

Integrated: 8263169: [macos] JavaFX windows open as tabs when system preference for documents is set

2021-04-06 Thread Kevin Rushforth
On Wed, 24 Mar 2021 13:38:27 GMT, Kevin Rushforth wrote: > If the macOS system settings for opening documents as tabs is set to > "Always", then all JavaFX windows, including Dialogs, are opened as tabs, > unless they are child windows (that is, a Window with an owner). > > This is a real

Re: RFR: 8263807: Button types of a DialogPane are set twice, returns a wrong button

2021-04-06 Thread Kevin Rushforth
On Tue, 6 Apr 2021 06:26:46 GMT, Marius Hanl wrote: >> I have one question on the fix (see below). Also, have you run all of the >> unit tests (not just the new one)? > >> >> >> I have one question on the fix (see below). Also, have you run all of the >> unit tests (not just the new one)? >

Re: RFR: 8263807: Button types of a DialogPane are set twice, returns a wrong button

2021-04-06 Thread Marius Hanl
On Mon, 5 Apr 2021 23:01:25 GMT, Kevin Rushforth wrote: > > > I have one question on the fix (see below). Also, have you run all of the > unit tests (not just the new one)? I did run all the normal tests and ever dialog related system test. Everything passed. Isn't jcheck running all the

Re: RFR: 8263169: [macos] JavaFX windows open as tabs when system preference for documents is set

2021-04-06 Thread Pankaj Bansal
On Wed, 24 Mar 2021 13:38:27 GMT, Kevin Rushforth wrote: > If the macOS system settings for opening documents as tabs is set to > "Always", then all JavaFX windows, including Dialogs, are opened as tabs, > unless they are child windows (that is, a Window with an owner). > > This is a real

Re: RFR: 8263807: Button types of a DialogPane are set twice, returns a wrong button

2021-04-06 Thread Marius Hanl
On Mon, 5 Apr 2021 22:58:47 GMT, Kevin Rushforth wrote: >> When DialogPane#getButtonTypes().setAll() is called twice with the same >> argument(s), DialogPane#lookupButton does not return the node which is shown >> inside the button bar. >> This is due DialogPane adding two list change