Re: RFR: 8088998: XYChart: duplicate child added exception when remove & add a series in several charts [v2]

2023-02-07 Thread Karthik P K
> While checking for duplicate series addition to the line chart, `setToRemove` > value was not considered before throwing exception. Hence code to handling > the case of adding the removed series was never run. > > Added condition to check `setToRemove` boolean value before throwing >

Re: RFR: 8290092: Temporary files are kept when call Clipboard.getSystemClipboard().getImage() [v3]

2023-02-07 Thread Ajit Ghaisas
On Tue, 7 Feb 2023 15:54:23 GMT, Lukasz Kostyra wrote: >> Windows implementation of GlassClipboard.cpp uses IStorage interface in >> `PopImage()` to allocate a temporary file, which is then used to capture >> Image data from system clipboard and move it to JVM side. In order for >> temporary

Re: [jfx20] RFR: 8301832: InputMethodEvents are not enabled for text input controls [v2]

2023-02-07 Thread Ajit Ghaisas
On Tue, 7 Feb 2023 16:24:34 GMT, Andy Goryachev wrote: >> **Targeting jfx20 branch** >> >> A fix for regression introduced by >> [JDK-8268877](https://bugs.openjdk.org/browse/JDK-8268877). >> >> For IME to work, both `setOnInputMethodTextChanged()` and >> `setInputMethodRequests()` needs to

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v5]

2023-02-07 Thread Kevin Rushforth
On Wed, 8 Feb 2023 00:12:14 GMT, Nir Lisker wrote: >> Fixes and cleanup in the areas in the linked issue. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Addressed review comments Marked as reviewed by kcr (Lead).

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v5]

2023-02-07 Thread John Hendrikx
On Wed, 8 Feb 2023 00:12:14 GMT, Nir Lisker wrote: >> Fixes and cleanup in the areas in the linked issue. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Addressed review comments Marked as reviewed by jhendrikx

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v5]

2023-02-07 Thread Andy Goryachev
On Wed, 8 Feb 2023 00:12:14 GMT, Nir Lisker wrote: >> Fixes and cleanup in the areas in the linked issue. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Addressed review comments definitely an improvement, thank you.

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v5]

2023-02-07 Thread Nir Lisker
> Fixes and cleanup in the areas in the linked issue. Nir Lisker has updated the pull request incrementally with one additional commit since the last revision: Addressed review comments - Changes: - all: https://git.openjdk.org/jfx/pull/1025/files - new:

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v4]

2023-02-07 Thread Nir Lisker
> Fixes and cleanup in the areas in the linked issue. Nir Lisker has updated the pull request incrementally with one additional commit since the last revision: Update modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java - Changes: - all:

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v4]

2023-02-07 Thread Nir Lisker
On Tue, 7 Feb 2023 19:52:14 GMT, Kevin Rushforth wrote: >> A specialized layout... +1 > > +1 from me as well. Suggestion: * A specialized layout for rich text. - PR: https://git.openjdk.org/jfx/pull/1025

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v3]

2023-02-07 Thread John Hendrikx
On Tue, 7 Feb 2023 20:54:20 GMT, John Hendrikx wrote: >> We aren't very consistent on this. Most of the places where we produce >> warnings, or where we provide verbose output, are done by printing. At a >> minimum, those should go to `System.err` rather than `System.out`. Unless >> there is

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v3]

2023-02-07 Thread John Hendrikx
On Tue, 7 Feb 2023 19:51:55 GMT, Kevin Rushforth wrote: >> I wish we were using logging... > > We aren't very consistent on this. Most of the places where we produce > warnings, or where we provide verbose output, are done by printing. At a > minimum, those should go to `System.err` rather

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v3]

2023-02-07 Thread Kevin Rushforth
On Tue, 7 Feb 2023 17:17:00 GMT, Nir Lisker wrote: >> modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java line >> 143: >> >>> 141: * application may set them to other values as needed: >>> 142: * >>> 143: * textflow.setMaxWidth(500); >> >> Maybe remove the `` tag

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v3]

2023-02-07 Thread Kevin Rushforth
On Tue, 7 Feb 2023 19:33:45 GMT, Andy Goryachev wrote: >> Maybe it's special in the sense that it's specialized towards displaying >> text, unlike other `Pane`s that are multi-purpose for all nodes. "designed >> to" already points to that. Maybe "A specialized layout for rich text."? > > A

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v3]

2023-02-07 Thread Kevin Rushforth
On Tue, 7 Feb 2023 19:31:58 GMT, Andy Goryachev wrote: >> I never understood when something should be logged vs. printed. And do we >> ever print to `out`? > > I wish we were using logging... We aren't very consistent on this. Most of the places where we produce warnings, or where we provide

Re: RFR: 8230833: LabeledSkinBase computes wrong height with ContentDisplay.GRAPHIC_ONLY [v4]

2023-02-07 Thread Andy Goryachev
On Tue, 7 Feb 2023 05:25:08 GMT, Karthik P K wrote: >> Ignore text condition was not considered in `computePrefHeight` method while >> calculating the Label height. >> >> Added `isIgnoreText` condition in `computePrefHeight` method while >> calculating Label height. >> >> Tests are present

Re: RFR: 8230833: LabeledSkinBase computes wrong height with ContentDisplay.GRAPHIC_ONLY [v4]

2023-02-07 Thread Andy Goryachev
On Tue, 7 Feb 2023 05:25:08 GMT, Karthik P K wrote: >> Ignore text condition was not considered in `computePrefHeight` method while >> calculating the Label height. >> >> Added `isIgnoreText` condition in `computePrefHeight` method while >> calculating Label height. >> >> Tests are present

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v3]

2023-02-07 Thread Andy Goryachev
On Tue, 7 Feb 2023 17:11:50 GMT, Nir Lisker wrote: >> modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java line >> 62: >> >>> 60: >>> 61: /** >>> 62: * A special layout designed to lay out rich text. >> >> What's "special" about it? :-) >> Suggestion: >> >> * A layout

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v3]

2023-02-07 Thread Andy Goryachev
On Tue, 7 Feb 2023 17:14:27 GMT, Nir Lisker wrote: >> modules/javafx.graphics/src/main/java/javafx/scene/Scene.java line 3277: >> >>> 3275: private void processDropEnd(DragEvent de) { >>> 3276: if (source == null) { >>> 3277:

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v3]

2023-02-07 Thread Andy Goryachev
On Tue, 7 Feb 2023 17:12:48 GMT, Nir Lisker wrote: >> Fixes and cleanup in the areas in the linked issue. > > Nir Lisker has updated the pull request incrementally with three additional > commits since the last revision: > > - Update >

Re: [jfx20] RFR: 8301832: InputMethodEvents are not enabled for text input controls [v2]

2023-02-07 Thread Kevin Rushforth
On Tue, 7 Feb 2023 16:24:34 GMT, Andy Goryachev wrote: >> **Targeting jfx20 branch** >> >> A fix for regression introduced by >> [JDK-8268877](https://bugs.openjdk.org/browse/JDK-8268877). >> >> For IME to work, both `setOnInputMethodTextChanged()` and >> `setInputMethodRequests()` needs to

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v3]

2023-02-07 Thread Nir Lisker
On Tue, 7 Feb 2023 13:05:21 GMT, Kevin Rushforth wrote: >> Nir Lisker has updated the pull request incrementally with three additional >> commits since the last revision: >> >> - Update >> modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java >> >>Co-authored-by: John

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v3]

2023-02-07 Thread Nir Lisker
On Tue, 7 Feb 2023 12:25:14 GMT, John Hendrikx wrote: >> Nir Lisker has updated the pull request incrementally with three additional >> commits since the last revision: >> >> - Update >> modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java >> >>Co-authored-by: John

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v3]

2023-02-07 Thread Nir Lisker
On Tue, 7 Feb 2023 12:17:43 GMT, Kevin Rushforth wrote: >> Nir Lisker has updated the pull request incrementally with three additional >> commits since the last revision: >> >> - Update >> modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java >> >>Co-authored-by: John

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v3]

2023-02-07 Thread Nir Lisker
> Fixes and cleanup in the areas in the linked issue. Nir Lisker has updated the pull request incrementally with three additional commits since the last revision: - Update modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java Co-authored-by: John Hendrikx - Update

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs [v2]

2023-02-07 Thread Nir Lisker
> Fixes and cleanup in the areas in the linked issue. Nir Lisker has updated the pull request incrementally with one additional commit since the last revision: Update modules/javafx.controls/src/main/java/javafx/scene/control/ListView.java Co-authored-by: John Hendrikx -

Re: RFR: 8173321: TableView: Click on right trough has no effect when cell height is higher than viewport height [v7]

2023-02-07 Thread JoachimSchriek
On Tue, 7 Feb 2023 12:08:19 GMT, Kevin Rushforth wrote: >> JoachimSchriek has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains seven additional >>

Re: [jfx20] RFR: 8301832: InputMethodEvents are not enabled for text input controls

2023-02-07 Thread Andy Goryachev
On Tue, 7 Feb 2023 12:11:58 GMT, Kevin Rushforth wrote: >> **Targeting jfx20 branch** >> >> A fix for regression introduced by >> [JDK-8268877](https://bugs.openjdk.org/browse/JDK-8268877). >> >> For IME to work, both `setOnInputMethodTextChanged()` and >> `setInputMethodRequests()` needs to

Re: [jfx20] RFR: 8301832: InputMethodEvents are not enabled for text input controls [v2]

2023-02-07 Thread Andy Goryachev
> **Targeting jfx20 branch** > > A fix for regression introduced by > [JDK-8268877](https://bugs.openjdk.org/browse/JDK-8268877). > > For IME to work, both `setOnInputMethodTextChanged()` and > `setInputMethodRequests()` needs to be set, see Scene:2242. > > The fix involves changing the code

Re: RFR: 8290092: Temporary files are kept when call Clipboard.getSystemClipboard().getImage() [v2]

2023-02-07 Thread Lukasz Kostyra
On Tue, 7 Feb 2023 11:50:21 GMT, Ajit Ghaisas wrote: >> Lukasz Kostyra has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - ClipboardExtImageTest: Add missing copyright notice >> - Add manual test to check for leftover Clipboard files > >

Re: RFR: 8290092: Temporary files are kept when call Clipboard.getSystemClipboard().getImage() [v3]

2023-02-07 Thread Lukasz Kostyra
> Windows implementation of GlassClipboard.cpp uses IStorage interface in > `PopImage()` to allocate a temporary file, which is then used to capture > Image data from system clipboard and move it to JVM side. In order for > temporary file to be removed automatically on `IStorage::Release()`, >

Integrated: 8290765: Remove parent disabled/treeVisible listeners

2023-02-07 Thread Michael Strauß
On Thu, 21 Jul 2022 04:43:15 GMT, Michael Strauß wrote: > `Node` adds InvalidationListeners to its parent's `disabled` and > `treeVisible` properties and calls its own `updateDisabled()` and > `updateTreeVisible(boolean)` methods when the property values change. > > These listeners are not

Re: Feedback requested for infrastructure for properties that wish to delay registering listeners

2023-02-07 Thread John Hendrikx
I've made a draft PR which shows the changes I'm proposing below. For those interested, it can be found here: https://github.com/openjdk/jfx/pull/1023 --John On 19/01/2023 16:49, John Hendrikx wrote: Hi list, I've been looking into what it would take to make the design of properties which

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs

2023-02-07 Thread Kevin Rushforth
On Mon, 6 Feb 2023 23:00:17 GMT, Nir Lisker wrote: > Fixes and cleanup in the areas in the linked issue. Other than John's comments, this looks good to me. I noted one more possible formatting change and an unrelated follow-up cleanup issue I plan to file.

Re: RFR: 8230833: LabeledSkinBase computes wrong height with ContentDisplay.GRAPHIC_ONLY [v4]

2023-02-07 Thread John Hendrikx
On Tue, 7 Feb 2023 05:25:08 GMT, Karthik P K wrote: >> Ignore text condition was not considered in `computePrefHeight` method while >> calculating the Label height. >> >> Added `isIgnoreText` condition in `computePrefHeight` method while >> calculating Label height. >> >> Tests are present

Re: [jfx20] RFR: 8293587: Fix mistakes in FX API docs

2023-02-07 Thread John Hendrikx
On Mon, 6 Feb 2023 23:00:17 GMT, Nir Lisker wrote: > Fixes and cleanup in the areas in the linked issue. Looks good to me, I added some more minor suggestions as well. modules/javafx.controls/src/main/java/javafx/scene/control/ListView.java line 467: > 465: /** > 466: * The {@code

Re: [jfx20] RFR: 8301832: InputMethodEvents are not enabled for text input controls

2023-02-07 Thread Ajit Ghaisas
On Mon, 6 Feb 2023 18:45:10 GMT, Andy Goryachev wrote: > **Targeting jfx20 branch** > > A fix for regression introduced by > [JDK-8268877](https://bugs.openjdk.org/browse/JDK-8268877). > > For IME to work, both `setOnInputMethodTextChanged()` and > `setInputMethodRequests()` needs to be set,

Re: [jfx20] RFR: 8301832: InputMethodEvents are not enabled for text input controls

2023-02-07 Thread Ajit Ghaisas
On Tue, 7 Feb 2023 12:11:58 GMT, Kevin Rushforth wrote: > This is the same category of bug fix as many accessibility fixes, in that it > requires entering a special mode on your computer to test it using an > existing test program like `HelloTextArea`. For this reason, I added a >

Re: [jfx20] RFR: 8301832: InputMethodEvents are not enabled for text input controls

2023-02-07 Thread Kevin Rushforth
On Mon, 6 Feb 2023 18:45:10 GMT, Andy Goryachev wrote: > **Targeting jfx20 branch** > > A fix for regression introduced by > [JDK-8268877](https://bugs.openjdk.org/browse/JDK-8268877). > > For IME to work, both `setOnInputMethodTextChanged()` and > `setInputMethodRequests()` needs to be set,

Re: RFR: 8173321: TableView: Click on right trough has no effect when cell height is higher than viewport height [v7]

2023-02-07 Thread Kevin Rushforth
On Mon, 6 Feb 2023 14:15:17 GMT, JoachimSchriek wrote: >> This is my (joachim.schr...@gmx.de) first contribution to openjfx. My >> Contributor Agreement is signed but still in review. >> So please be patient with an absolute beginner as contributor ... . >> The work of this pull request was

Re: RFR: 8290092: Temporary files are kept when call Clipboard.getSystemClipboard().getImage() [v2]

2023-02-07 Thread Ajit Ghaisas
On Thu, 2 Feb 2023 16:44:11 GMT, Lukasz Kostyra wrote: >> Windows implementation of GlassClipboard.cpp uses IStorage interface in >> `PopImage()` to allocate a temporary file, which is then used to capture >> Image data from system clipboard and move it to JVM side. In order for >> temporary

Re: [jfx20] RFR: 8301832: InputMethodEvents are not enabled for text input controls

2023-02-07 Thread Ajit Ghaisas
On Mon, 6 Feb 2023 18:45:10 GMT, Andy Goryachev wrote: > **Targeting jfx20 branch** > > A fix for regression introduced by > [JDK-8268877](https://bugs.openjdk.org/browse/JDK-8268877). > > For IME to work, both `setOnInputMethodTextChanged()` and > `setInputMethodRequests()` needs to be set,

Re: RFR: 8290765: Remove parent disabled/treeVisible listeners [v5]

2023-02-07 Thread Ambarish Rapte
On Fri, 3 Feb 2023 23:31:24 GMT, Michael Strauß wrote: >> `Node` adds InvalidationListeners to its parent's `disabled` and >> `treeVisible` properties and calls its own `updateDisabled()` and >> `updateTreeVisible(boolean)` methods when the property values change. >> >> These listeners are