Re: RFR: 8242553: IntegerSpinner and DoubleSpinner do not wrap around values correctly in some cases

2024-03-27 Thread Jeanette Winzenburg
On Wed, 27 Mar 2024 15:17:29 GMT, Andy Goryachev wrote: > (0 + 101) % (100 - 0) + 0 = 1 > > the code in this PR produces no movement (0). Same for the decrement. > which means that the modulo arithmetic is not yet quite correct (_not_ that the modulo behavior as such is wrong ;) nice test c

Re: RFR: 8242553: IntegerSpinner and DoubleSpinner do not wrap around values correctly in some cases

2024-03-27 Thread Jeanette Winzenburg
On Sun, 24 Mar 2024 15:11:16 GMT, drmarmac wrote: > This PR should fix the issue and cover all relevant cases with new tests. > > Note: This involves a small behavior change, as can be seen in > dblSpinner_testWrapAround_decrement_twoSteps() in SpinnerTest.java:749. With > this change the wrap

Re: RFR: 8321970: New table columns don't appear when using fixed cell size unless refreshing tableView

2023-12-18 Thread Jeanette Winzenburg
On Mon, 18 Dec 2023 12:09:21 GMT, Jose Pereda wrote: > This PR fixes an issue when a new `TableColumn` is added to a `TableView` > control with fixed cell size set, where the `TableRowSkinBase` failed to add > the cells for the new column. > > A test is included that fails before applying this

Re: RFR: 8316135: Create release notes for JavaFX 21 [v3]

2023-09-16 Thread Jeanette Winzenburg
On Wed, 13 Sep 2023 12:06:44 GMT, Kevin Rushforth wrote: >> Release notes for JavaFX 21, including four important changes, and the list >> of enhancements and bugs fixed in this release. >> >> I plan to integrate this on Monday, Sep 18th, and backport it to `jfx21` in >> time for Tuesday's rel

Re: RFR: 8285700: [TreeTableView] graphic property of TreeItem is still visible after collapsing tree

2023-07-11 Thread Jeanette Winzenburg
On Tue, 11 Jul 2023 12:38:39 GMT, Karthik P K wrote: > > Is it possible to write a Unit test as well? > > I tried to write unit test but since it should be checked if the graphic is > cleared while collapsing the tree and added back while expanding, I couldn't > find out a way to do this. I

Re: CFV: New OpenJFX Reviewer: Jose Pereda

2023-02-17 Thread Jeanette Winzenburg
Vote: yes Zitat von Kevin Rushforth : I hereby nominate Jose Pereda [1] to OpenJFX Reviewer. Jose is an OpenJFX community member, who has contributed 59 commits [2][3] to OpenJFX. Votes are due by March 2, 2023 at 23:59 UTC. Only current OpenJFX Reviewers [4] are eligible to vote on this

Re: RFR: 8187145: (Tree)TableView with null selectionModel: throws NPE on sorting [v5]

2022-11-04 Thread Jeanette Winzenburg
On Fri, 4 Nov 2022 05:07:17 GMT, Ajit Ghaisas wrote: >> There is a `return null` on line 176. >> In that case, we will return from either line 173 or line 176. Hence, there >> is no fall through. > > Any comments on this? hmm .. maybe this is a mis-understanding: - the case selected_items look

Re: RFR: 8268877: TextInputControlSkin: incorrect inputMethod event handler after switching skin

2022-11-03 Thread Jeanette Winzenburg
On Tue, 27 Sep 2022 19:36:46 GMT, Andy Goryachev wrote: > Using new Skin.install() method to properly install and uninstall > inputMethodTextChanged and inputMethodRequests properties on TextInputControl. > > This avoids memory leaks resulting from skin change, as well as honors > user-set pro

Re: RFR: 8290844: Add Skin.install() method [v9]

2022-10-05 Thread Jeanette Winzenburg
On Tue, 4 Oct 2022 19:22:42 GMT, Andy Goryachev wrote: >> so maybe `setting the {@link #skinProperty() skin property}`? > > please check the updated comment, I think it sounds weird... that change sounds okay to me :) What might be missing is a description of that it actually does, that is tak

Re: RFR: 8290844: Add Skin.install() method [v9]

2022-10-05 Thread Jeanette Winzenburg
On Tue, 4 Oct 2022 19:27:55 GMT, Andy Goryachev wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/Skin.java line 90: >> >>> 88: */ >>> 89: default public void install() { } >>> 90: >> >> what about calling install more than once? There are arguments for either: >>

Re: RFR: 8290844: Add Skin.install() method [v9]

2022-10-05 Thread Jeanette Winzenburg
On Tue, 4 Oct 2022 19:24:56 GMT, Andy Goryachev wrote: >> I don't think we would want to go out of our way to enable this, so I prefer >> the tighter definition of the life-cycle that Andy is proposing. It seems >> better to have the Control always call `dispose` and `install` rather than >> p

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin

2022-10-05 Thread Jeanette Winzenburg
On Wed, 5 Oct 2022 11:11:46 GMT, Jeanette Winzenburg wrote: >> this makes no sense! thank you for pointing it out. >> perhaps we really *ought to* create a better >> LambdaMultiplePropertyChangeListenerHandler implementation. > > well, it _does_ make sense for

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin

2022-10-05 Thread Jeanette Winzenburg
On Tue, 4 Oct 2022 20:29:05 GMT, Andy Goryachev wrote: > I have a string feeling that this PR should go back to DRAFT state, pending a > better listener helper implementation JDK-8294809. Also, this particular skin > may need to get it's listeners refactored - I see two scene listeners which >

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin

2022-10-05 Thread Jeanette Winzenburg
On Tue, 4 Oct 2022 20:15:47 GMT, Andy Goryachev wrote: >> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/LambdaMultiplePropertyChangeListenerHandler.java >> line 183: >> >>> 181: consumer.accept(v); >>> 182: } >>> 183: }); >>

Re: RFR: 8290844: Add Skin.install() method [v9]

2022-10-04 Thread Jeanette Winzenburg
On Thu, 29 Sep 2022 15:58:01 GMT, Andy Goryachev wrote: >> - added Skin.install() >> - javadoc changes for Skinnable.setSkin(Skin) > > Andy Goryachev 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

Re: RFR: 8290844: Add Skin.install() method [v9]

2022-10-04 Thread Jeanette Winzenburg
On Thu, 29 Sep 2022 15:58:01 GMT, Andy Goryachev wrote: >> - added Skin.install() >> - javadoc changes for Skinnable.setSkin(Skin) > > Andy Goryachev 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

Re: RFR: 8290844: Add Skin.install() method [v9]

2022-10-04 Thread Jeanette Winzenburg
On Thu, 29 Sep 2022 15:58:01 GMT, Andy Goryachev wrote: >> - added Skin.install() >> - javadoc changes for Skinnable.setSkin(Skin) > > Andy Goryachev 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

Re: RFR: 8290844: Add Skin.install() method [v9]

2022-10-04 Thread Jeanette Winzenburg
On Thu, 29 Sep 2022 15:58:01 GMT, Andy Goryachev wrote: >> - added Skin.install() >> - javadoc changes for Skinnable.setSkin(Skin) > > Andy Goryachev 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

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin

2022-10-04 Thread Jeanette Winzenburg
On Thu, 29 Sep 2022 23:00:17 GMT, Andy Goryachev wrote: > Fixed memory leak by using weak listeners and making sure to undo everything > that has been done to MenuBar/Skin in dispose(). modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuBarSkin.java line 328: > 326: m

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin

2022-10-04 Thread Jeanette Winzenburg
On Thu, 29 Sep 2022 23:00:17 GMT, Andy Goryachev wrote: > Fixed memory leak by using weak listeners and making sure to undo everything > that has been done to MenuBar/Skin in dispose(). modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuBarSkin.java line 950: > 948: > 949:

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin

2022-10-04 Thread Jeanette Winzenburg
On Thu, 29 Sep 2022 23:00:17 GMT, Andy Goryachev wrote: > Fixed memory leak by using weak listeners and making sure to undo everything > that has been done to MenuBar/Skin in dispose(). modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/LambdaMultiplePropertyChangeListenerHandle

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin

2022-10-04 Thread Jeanette Winzenburg
On Tue, 4 Oct 2022 14:20:46 GMT, Jeanette Winzenburg wrote: >>> Thanks again, @kleopatra With your permission, I'll add tests with and >>> without scene property set. Or do we want to keep the original set? >> >> SkinMemoryLeakTest already has both metho

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin

2022-10-04 Thread Jeanette Winzenburg
On Tue, 4 Oct 2022 14:06:59 GMT, Jeanette Winzenburg wrote: >>> Perhaps the test is too artificial, something is not being done correctly >>> or exactly as in the real application? Using StageLoader or showControl() >>> hooks up the missing dependencies. >>

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin

2022-10-04 Thread Jeanette Winzenburg
On Fri, 30 Sep 2022 16:21:32 GMT, Jeanette Winzenburg wrote: >> Will definitely do! Some tests were failing yesterday, until all is fixed - >> it's a draft PR :-) >> Thank you so much, @kleopatra > >> Perhaps the test is too artificial, something is not being

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin

2022-10-03 Thread Jeanette Winzenburg
On Fri, 30 Sep 2022 16:19:33 GMT, Andy Goryachev wrote: >> okay, not tested (so treat it as just a wild guess :) - there is this >> listener on the skinnable's sceneProperty: it's installed only when there's >> no scene and removed once the scene is set. Without showing, it looks like >> it's

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin

2022-10-03 Thread Jeanette Winzenburg
On Fri, 30 Sep 2022 16:05:57 GMT, Andy Goryachev wrote: >> after: >> > src="https://user-images.githubusercontent.com/107069028/193310795-48b2a72d-aad7-4c36-81d8-740f302fccb7.png";> > > See > https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java okay, not test

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin

2022-10-03 Thread Jeanette Winzenburg
On Fri, 30 Sep 2022 15:51:55 GMT, Jeanette Winzenburg wrote: >> So here we have a problem - I know that the memory leak is fixed (I've >> tested using VisualVM in a real application), but without this change, or >> without adding a StageLoader, the test fails. >>

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin

2022-10-03 Thread Jeanette Winzenburg
On Fri, 30 Sep 2022 15:06:19 GMT, Andy Goryachev wrote: >> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinMemoryLeakTest.java >> line 94: >> >>> 92: public void testMemoryLeakSameSkinClass() { >>> 93: showControl(control, true); >>> 94: installDefau

Re: RFR: 8294589: MenuBarSkin: memory leak when changing skin

2022-10-03 Thread Jeanette Winzenburg
On Thu, 29 Sep 2022 23:00:17 GMT, Andy Goryachev wrote: > Fixed memory leak by using weak listeners and making sure to undo everything > that has been done to MenuBar/Skin in dispose(). modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinMemoryLeakTest.java line 94: > 92:

Re: RFR: 8292353: TableRow vs. TreeTableRow: inconsistent visuals in cell selection mode [v12]

2022-09-23 Thread Jeanette Winzenburg
On Tue, 20 Sep 2022 19:06:19 GMT, Andy Goryachev wrote: >> The issue is caused by TreeTableRow incorrectly selected when cell selection >> mode is enabled. >> >> Changes: >> - modified TreeTableRow.updateSelection() > > Andy Goryachev has updated the pull request incrementally with one addition

Re: RFR: 8279640: ListView with null SelectionModel/FocusModel throws NPE [v5]

2022-09-14 Thread Jeanette Winzenburg
On Mon, 12 Sep 2022 16:27:06 GMT, Marius Hanl wrote: >> This PR fixes a bunch of NPEs when a null `SelectionModel` or `FocusModel` >> is set on a `ListView`. >> >> The following NPEs are fixed (all are also covered by exactly one test case): >> NPEs with null selection model: >> - Mouse click o

Re: RFR: 8279640: ListView with null SelectionModel/FocusModel throws NPE [v5]

2022-09-14 Thread Jeanette Winzenburg
On Sun, 11 Sep 2022 19:29:05 GMT, Marius Hanl wrote: > > Personally, I think that we should support both - we have a separate > focusModel so should use it independently of the selectionModel where > possible. A good starter for this issue might be to implement the activate > such that it doe

Re: RFR: 8292353: TableRow vs. TreeTableRow: inconsistent visuals in cell selection mode [v10]

2022-09-14 Thread Jeanette Winzenburg
On Tue, 13 Sep 2022 21:10:44 GMT, Andy Goryachev wrote: > Is it possible to have two or more rows with the same row index? i would > imagine that will break a lot of things. don't imagine, verify ;) What happens when you run the test? What happens when you look at the scenegraph (f.i. with Sce

Re: RFR: 8089280: horizontal scrollbar should never become visible in TableView with constrained resize policy [v2]

2022-09-12 Thread Jeanette Winzenburg
On Mon, 12 Sep 2022 11:46:11 GMT, Kevin Rushforth wrote: > > I hadn't tried running `HorizontalConstrainedTableScrollingMin` on Mac, since > Andy said he couldn't reproduce it with that program. I just now ran it, and > no, that specific program doesn't show the bug on Mac. However, the unit t

Re: RFR: 8089280: horizontal scrollbar should never become visible in TableView with constrained resize policy [v2]

2022-09-11 Thread Jeanette Winzenburg
On Sat, 10 Sep 2022 15:15:01 GMT, Kevin Rushforth wrote: > The fix looks good to me. I verified that the new tests fail without the fix > and pass with the fix. I was also able to reproduce the problem using the > [HorizontalConstrainedTableScrollingMin > test](https://bugs.openjdk.org/browse/

Re: RFR: 8292353: TableRow vs. TreeTableRow: inconsistent visuals in cell selection mode [v10]

2022-09-06 Thread Jeanette Winzenburg
On Wed, 24 Aug 2022 22:03:26 GMT, Andy Goryachev wrote: >> The issue is caused by TreeTableRow incorrectly selected when cell selection >> mode is enabled. >> >> Changes: >> - modified TreeTableRow.updateSelection() > > Andy Goryachev has updated the pull request incrementally with one addition

Re: RFR: 8292353: TableRow vs. TreeTableRow: inconsistent visuals in cell selection mode [v10]

2022-09-06 Thread Jeanette Winzenburg
On Wed, 24 Aug 2022 22:03:26 GMT, Andy Goryachev wrote: >> The issue is caused by TreeTableRow incorrectly selected when cell selection >> mode is enabled. >> >> Changes: >> - modified TreeTableRow.updateSelection() > > Andy Goryachev has updated the pull request incrementally with one addition

Re: RFR: 8290844: Add Skin.install() method [v4]

2022-09-03 Thread Jeanette Winzenburg
On Fri, 12 Aug 2022 18:15:37 GMT, Andy Goryachev wrote: >> - added Skin.install() >> - javadoc changes for Skinnable.setSkin(Skin) >> >> no code changes for Skinnable.setSkin(Skin) yet. > > Andy Goryachev has updated the pull request incrementally with one additional > commit since the last rev

Re: RFR: 8290844: Add Skin.install() method [v4]

2022-08-24 Thread Jeanette Winzenburg
On Fri, 12 Aug 2022 18:15:37 GMT, Andy Goryachev wrote: >> - added Skin.install() >> - javadoc changes for Skinnable.setSkin(Skin) >> >> no code changes for Skinnable.setSkin(Skin) yet. > > Andy Goryachev has updated the pull request incrementally with one additional > commit since the last rev

Re: RFR: 8290844: Add Skin.install() method [v4]

2022-08-24 Thread Jeanette Winzenburg
On Fri, 12 Aug 2022 18:15:37 GMT, Andy Goryachev wrote: >> - added Skin.install() >> - javadoc changes for Skinnable.setSkin(Skin) >> >> no code changes for Skinnable.setSkin(Skin) yet. > > Andy Goryachev has updated the pull request incrementally with one additional > commit since the last rev

Re: RFR: 8290844: Add Skin.install() method [v3]

2022-08-24 Thread Jeanette Winzenburg
On Wed, 24 Aug 2022 15:29:01 GMT, Andy Goryachev wrote: > the constructor cannot distinguish between this property set by the user true, but currently it doesn't even _try_ to. So IMO that's __not__ a problematic pattern (as already pointed out in our recent debate): the install sets whatever

Re: RFR: 8290844: Add Skin.install() method [v3]

2022-08-24 Thread Jeanette Winzenburg
On Mon, 15 Aug 2022 15:37:15 GMT, Jeanette Winzenburg wrote: >>> A quick PoC is in [my fork off Andy's >>> PR](https://github.com/kleopatra/jfx/tree/exp-skin-install-supportsInstall) >> >> @kleopatra : >> Thank you so much for your effort to researc

Re: RFR: 8292353: TableRow vs. TreeTableRow: inconsistent visuals in cell selection mode [v6]

2022-08-24 Thread Jeanette Winzenburg
On Tue, 23 Aug 2022 19:58:24 GMT, Andy Goryachev wrote: > > I do agree that, in general, it might be beneficial to have one test case per > condition or failure, but it should not be a requirement. well, this project follows (should, at least, there are legacy areas ;) established best pract

Re: RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v11]

2022-08-23 Thread Jeanette Winzenburg
On Fri, 12 Aug 2022 17:21:24 GMT, Andy Goryachev wrote: >> 1. reword SelectionModel.isSelected(int) javadoc, removing incorrect >> statement "Is functionally equivalent to calling >> getSelectedIndices().contains(index)." >> 2. reimplement TableView.TableViewSelectionModel.isSelected(int) meth

Re: RFR: 8292353: TableRow vs. TreeTableRow: inconsistent visuals in cell selection mode [v6]

2022-08-23 Thread Jeanette Winzenburg
On Mon, 22 Aug 2022 21:08:39 GMT, Andy Goryachev wrote: >> The issue is caused by TreeTableRow incorrectly selected when cell selection >> mode is enabled. >> >> Changes: >> - modified TreeTableRow.updateSelection() > > Andy Goryachev has updated the pull request incrementally with two addition

Re: RFR: 8279640: ListView with null SelectionModel/FocusModel throws NPE

2022-08-22 Thread Jeanette Winzenburg
On Mon, 22 Aug 2022 15:43:28 GMT, Jeanette Winzenburg wrote: >> There is also an unguarded access in CellBehaviorBase.selectRows(int,int) : >> 312 and CellBehaviorBase.sompleSelect(MouseButton,int,boolean) : 274, which >> are being fixed by #876 >> >> I

Re: RFR: 8279640: ListView with null SelectionModel/FocusModel throws NPE

2022-08-22 Thread Jeanette Winzenburg
On Mon, 22 Aug 2022 15:29:29 GMT, Andy Goryachev wrote: > There is also an unguarded access in CellBehaviorBase.selectRows(int,int) : > 312 and CellBehaviorBase.sompleSelect(MouseButton,int,boolean) : 274, which > are being fixed by #876 > > I'd suggest to integrate #876 first, followed by thi

Re: RFR: 8279640: ListView with null SelectionModel/FocusModel throws NPE

2022-08-21 Thread Jeanette Winzenburg
On Sat, 8 Jan 2022 00:17:36 GMT, Marius Hanl wrote: > This PR fixes a bunch of NPEs when a null `SelectionModel` or `FocusModel` is > set on a `ListView`. > > The following NPEs are fixed (all are also covered by exactly one test case): > NPEs with null selection model: > - Mouse click on a `Li

Re: RFR: 8187145: (Tree)TableView with null selectionModel: throws NPE on sorting

2022-08-19 Thread Jeanette Winzenburg
On Thu, 18 Aug 2022 16:24:11 GMT, Andy Goryachev wrote: > Setting a null selection model in TableView and TreeTableView produce NPE on > sorting (and probably in some other situations) because the check for null is > missing in several places. > > Setting a null selection model is a valid way

Re: RFR: 8292353: TableRow vs. TreeTableRow: inconsistent visuals in cell selection mode

2022-08-18 Thread Jeanette Winzenburg
On Wed, 17 Aug 2022 19:40:21 GMT, Andy Goryachev wrote: > The issue is caused by TreeTableRow incorrectly selected when cell selection > mode is enabled. > > Changes: > - modified TreeTableRow.updateSelection() Changes requested by fastegal (Reviewer). modules/javafx.controls/src/main/java/ja

Re: RFR: 8290844: Add Skin.install() method [v3]

2022-08-15 Thread Jeanette Winzenburg
On Fri, 12 Aug 2022 18:42:16 GMT, Andy Goryachev wrote: > > @kleopatra : Thank you so much for your effort to research the alternatives. > thinking about alternatives is our job, isn't it :) > The main issue that necessitates creation of install() and moving it outside > of the skin's constr

Re: RFR: 8290844: Add Skin.install() method [v4]

2022-08-15 Thread Jeanette Winzenburg
On Fri, 12 Aug 2022 18:15:37 GMT, Andy Goryachev wrote: >> - added Skin.install() >> - javadoc changes for Skinnable.setSkin(Skin) >> >> no code changes for Skinnable.setSkin(Skin) yet. > > Andy Goryachev has updated the pull request incrementally with one additional > commit since the last rev

Re: RFR: 8290473: update Eclipse .classpath in apps, buildSrc [v9]

2022-08-15 Thread Jeanette Winzenburg
On Fri, 12 Aug 2022 23:23:46 GMT, Andy Goryachev wrote: >> The goal of this change is to make sure jfx repo can be imported as a gradle >> project in eclipse and all nested projects in the workspace compile with no >> errors. >> >> - updated .classpath entries in apps/ >> - added utf-8 prefs i

Merge master into fix branch during review

2022-08-15 Thread Jeanette Winzenburg
.. is something I _personally_ don't like: have to mentally sort the related from the unrelated commits in the history. The contributing guidelines do allow intermediate merges (bolding by me) "If you __need__ to pick up changes from master, you can merge master into your branch" my int

Re: RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v11]

2022-08-15 Thread Jeanette Winzenburg
On Fri, 12 Aug 2022 17:21:24 GMT, Andy Goryachev wrote: >> 1. reword SelectionModel.isSelected(int) javadoc, removing incorrect >> statement "Is functionally equivalent to calling >> getSelectedIndices().contains(index)." >> 2. reimplement TableView.TableViewSelectionModel.isSelected(int) meth

Re: RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v10]

2022-08-15 Thread Jeanette Winzenburg
On Thu, 11 Aug 2022 17:56:58 GMT, Andy Goryachev wrote: > unnecessary in the case of TableRow. that is .. why exactly? > the change in TreeTableRow was necessary because otherwise selecting a cell > by mouse selected the whole row. it still does (if all cells are selected) see me in grinnin

Re: RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v7]

2022-08-15 Thread Jeanette Winzenburg
On Fri, 12 Aug 2022 15:08:27 GMT, Andy Goryachev wrote: > > you are right... I am so sorry. missed this one while cherry picking from > another branch. will fix it soon. no problem, shit happens :) - PR: https://git.openjdk.org/jfx/pull/839

Re: RFR: 8290844: Add Skin.install() method [v3]

2022-08-12 Thread Jeanette Winzenburg
On Thu, 11 Aug 2022 22:19:39 GMT, Andy Goryachev wrote: >> - added Skin.install() >> - javadoc changes for Skinnable.setSkin(Skin) >> >> no code changes for Skinnable.setSkin(Skin) yet. > > Andy Goryachev has updated the pull request with a new target base due to a > merge or a rebase. The incr

Re: RFR: 8290844: Add Skin.install() method [v2]

2022-08-12 Thread Jeanette Winzenburg
On Thu, 11 Aug 2022 23:24:11 GMT, Kevin Rushforth wrote: > > How common do you think this sort of thing is in practice? can't do more than guessing (knowing that companies I worked for had done so before I got my hands onto their code) - so would say it's common enough to cause .. well, some

Re: Proposal: Add Skin.install() method

2022-08-12 Thread Jeanette Winzenburg
are not yet handled (most don't even have an issue yet). -- Jeanette Thank you so much for your comments! -andy From: openjfx-dev on behalf of Jeanette Winzenburg Date: Thursday, 2022/07/21 at 09:04 To: openjfx-dev@openjdk.org Subject: Re: Proposal: Add Skin.install() metho

Re: RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v7]

2022-08-12 Thread Jeanette Winzenburg
On Thu, 11 Aug 2022 16:14:08 GMT, Andy Goryachev wrote: > > The tests were added to both TableViewSelectionModelImplTest and > TreeTableViewSelectionModelImplTest. hmm .. in TreeTableViewSelectionModelImplTest, I don't see any of the testRowSelectionAfterSelectAndHideLastColumnXX nor testSe

Re: RFR: 8290844: Add Skin.install() method [v2]

2022-08-11 Thread Jeanette Winzenburg
On Wed, 10 Aug 2022 22:14:53 GMT, Andy Goryachev wrote: >> - added Skin.install() >> - javadoc changes for Skinnable.setSkin(Skin) >> >> no code changes for Skinnable.setSkin(Skin) yet. > > Andy Goryachev has updated the pull request incrementally with one additional > commit since the last rev

Re: RFR: 8290844: Add Skin.install() method [v2]

2022-08-11 Thread Jeanette Winzenburg
On Wed, 10 Aug 2022 22:14:53 GMT, Andy Goryachev wrote: >> - added Skin.install() >> - javadoc changes for Skinnable.setSkin(Skin) >> >> no code changes for Skinnable.setSkin(Skin) yet. > > Andy Goryachev has updated the pull request incrementally with one additional > commit since the last rev

Re: RFR: 8290844: Add Skin.install() method [v2]

2022-08-11 Thread Jeanette Winzenburg
On Wed, 10 Aug 2022 22:14:53 GMT, Andy Goryachev wrote: >> - added Skin.install() >> - javadoc changes for Skinnable.setSkin(Skin) >> >> no code changes for Skinnable.setSkin(Skin) yet. > > Andy Goryachev has updated the pull request incrementally with one additional > commit since the last rev

Re: RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v7]

2022-08-11 Thread Jeanette Winzenburg
On Tue, 9 Aug 2022 11:51:09 GMT, Jeanette Winzenburg wrote: > the tests - they deserve to be integrated into > TableViewSelectionModelImplTest, with your permission. similar tests (along with that in the bug report) adapted to treeTable should be added to the treeTable test - we shoul

Re: RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v10]

2022-08-11 Thread Jeanette Winzenburg
On Wed, 10 Aug 2022 15:33:15 GMT, Andy Goryachev wrote: >> 1. reword SelectionModel.isSelected(int) javadoc, removing incorrect >> statement "Is functionally equivalent to calling >> getSelectedIndices().contains(index)." >> 2. reimplement TableView.TableViewSelectionModel.isSelected(int) meth

Re: RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v8]

2022-08-09 Thread Jeanette Winzenburg
On Mon, 8 Aug 2022 22:07:17 GMT, Andy Goryachev wrote: > > Personally, I feel that any manipulations with the columns structure should > clear the existing selection. I would imagine it's such a rare operation, and > the "fix" is so easy (clear selection) that it's not worth even creating an

Re: RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v7]

2022-08-09 Thread Jeanette Winzenburg
On Mon, 8 Aug 2022 21:51:05 GMT, Andy Goryachev wrote: > > I think you are right, @kleopatra ! Took me a while, but I see your point > now. This method should indeed delegate to super(), or not be overridden at > all. > I would prefer removing it (reducing clutter :) As it's a private class,

Re: RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v4]

2022-08-06 Thread Jeanette Winzenburg
On Thu, 28 Jul 2022 16:37:33 GMT, Andy Goryachev wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/TableView.java >> line 2829: >> >>> 2827: return selectedCellsMap.isSelected(index, -1); >>> 2828: } >>> 2829: } >> >> wondering why you dis

Re: RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v6]

2022-08-06 Thread Jeanette Winzenburg
On Wed, 3 Aug 2022 23:28:24 GMT, Andy Goryachev wrote: >> 1. reword SelectionModel.isSelected(int) javadoc, removing incorrect >> statement "Is functionally equivalent to calling >> getSelectedIndices().contains(index)." >> 2. reimplement TableView.TableViewSelectionModel.isSelected(int) metho

Re: JBS: broken commit references for old (fx8) fixes

2022-08-06 Thread Jeanette Winzenburg
that's working, thanks Kevin :) Zitat von Kevin Rushforth : There is no automatic redirect, but you can replace 8/graphics/rt with 8u-dev/rt using the same hash: https://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/6d1c6c47a5dd -- Kevin On 8/6/2022 5:56 AM, Jeanette Winzenburg

JBS: broken commit references for old (fx8) fixes

2022-08-06 Thread Jeanette Winzenburg
an example is https://bugs.openjdk.org/browse/JDK-8123472 (was: RT-33442) - it's fixed Oct. 2013 (after the history of the current repository starts, which is Nov. 2011), the commit ref is http://hg.openjdk.java.net/openjfx/8/graphics/rt/rev/6d1c6c47a5dd which opens an error page: "An e

Re: RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v6]

2022-08-06 Thread Jeanette Winzenburg
On Fri, 5 Aug 2022 14:07:29 GMT, Kevin Rushforth wrote: > > The specification section of the CSR should (and does) list only the spec > change to `SelectionModel.isSelected`. Since there could be an impact to > applications that rely on the current behavior, it also seems reasonable to > list

Re: RFR: 8290473: update Eclipse .classpath in apps, buildSrc [v2]

2022-08-05 Thread Jeanette Winzenburg
On Fri, 5 Aug 2022 11:02:50 GMT, Jeanette Winzenburg wrote: >> This is puzzling, @kleopatra . >> I don't see a reference to [hlsl/]Prism or [hlsl/]Decora anywhere. Not in >> any gradle.build file, not in every file. I don't think these two >> directories

Re: RFR: 8290473: update Eclipse .classpath in apps, buildSrc [v2]

2022-08-05 Thread Jeanette Winzenburg
On Thu, 4 Aug 2022 17:07:53 GMT, Andy Goryachev wrote: >> the tests are fine - it happens if I have a separate project and let that >> depend on the controls, basic, graphics projects (just the same setup as >> described on the mailinglist) > > This is puzzling, @kleopatra . > I don't see a re

Re: RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v6]

2022-08-05 Thread Jeanette Winzenburg
On Fri, 5 Aug 2022 10:44:55 GMT, Jeanette Winzenburg wrote: >> Andy Goryachev 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 cont

Re: RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v6]

2022-08-05 Thread Jeanette Winzenburg
On Wed, 3 Aug 2022 23:28:24 GMT, Andy Goryachev wrote: >> 1. reword SelectionModel.isSelected(int) javadoc, removing incorrect >> statement "Is functionally equivalent to calling >> getSelectedIndices().contains(index)." >> 2. reimplement TableView.TableViewSelectionModel.isSelected(int) metho

Re: RFR: 8290473: update Eclipse .classpath in apps, buildSrc

2022-08-04 Thread Jeanette Winzenburg
On Thu, 4 Aug 2022 15:33:11 GMT, Andy Goryachev wrote: >> modules/javafx.graphics/.classpath line 5: >> >>> 3: >>> 4: >>> 5: >> >> these two seem not enough for running projects that depend on the graphics >> project, without the other two (from the original before the [PR >> 804](https

Re: RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v2]

2022-08-04 Thread Jeanette Winzenburg
On Wed, 27 Jul 2022 05:47:56 GMT, Ajit Ghaisas wrote: >> thank you for your comments, Ajit! below are responses, please let me know >> if you agree or not: >> >> 1. javadoc for TableSelectionModel.isSelected(int, TablecolumnBase) already >> describes the logic: >> ` >>/** >> * Conven

Re: RFR: 8290473: update Eclipse .classpath in apps, buildSrc

2022-08-04 Thread Jeanette Winzenburg
On Mon, 1 Aug 2022 16:53:29 GMT, Andy Goryachev wrote: > The goal of this change is to make sure jfx repo can be imported as a gradle > project in eclipse and all nested projects in the workspace compile with no > errors. > > - updated .classpath entries in apps/ > - added utf-8 prefs in .sett

Re: RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v4]

2022-07-28 Thread Jeanette Winzenburg
On Wed, 27 Jul 2022 21:36:10 GMT, Andy Goryachev wrote: >> 1. reword SelectionModel.isSelected(int) javadoc, removing incorrect >> statement "Is functionally equivalent to calling >> getSelectedIndices().contains(index)." >> 2. reimplement TableView.TableViewSelectionModel.isSelected(int) meth

Re: RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v2]

2022-07-28 Thread Jeanette Winzenburg
On Thu, 21 Jul 2022 21:10:10 GMT, Andy Goryachev wrote: >> 1. reword SelectionModel.isSelected(int) javadoc, removing incorrect >> statement "Is functionally equivalent to calling >> getSelectedIndices().contains(index)." >> 2. reimplement TableView.TableViewSelectionModel.isSelected(int) meth

Re: Proposal: Add Skin.install() method

2022-07-21 Thread Jeanette Winzenburg
Zitat von Andy Goryachev : Hi Andy, good idea to move our discussion from the issues to the mailling - it's both easier and reaches a broader audience :) Will answer in about a week or so, for now just stumbled across We can see the damage caused by looking at JDK-8241364

Re: Eclipse: ClassNotFoundException: com.sun.prism.shader.FillPgram_Color_Loader

2022-07-15 Thread Jeanette Winzenburg
Zitat von Nir Lisker : What resource is the error on? what do you mean by _resource_? It's on the console .. On Fri, Jul 15, 2022 at 1:19 PM Jeanette Winzenburg wrote: Zitat von Nir Lisker : Hi Nir, thanks for the explanation - though I have no idea why Eclipse wants them

Re: Eclipse: ClassNotFoundException: com.sun.prism.shader.FillPgram_Color_Loader

2022-07-15 Thread Jeanette Winzenburg
t files were updated recently in https://github.com/openjdk/jfx/pull/804. The OS-specific folders were removed. I tested it and I had no issue after this change. On what resource are you getting this error? On Fri, Jul 15, 2022 at 12:44 PM Jeanette Winzenburg < faste...@swingempire.de> wrote: Hi,

Re: Eclipse: ClassNotFoundException: com.sun.prism.shader.FillPgram_Color_Loader

2022-07-15 Thread Jeanette Winzenburg
, sry ;) But just noticed that reverting the change in .classpath in graphics seemed to fix it .. On Fri, Jul 15, 2022 at 12:44 PM Jeanette Winzenburg < faste...@swingempire.de> wrote: Hi, after synching my master branch with upstream (has been a long while since my previous update

Eclipse: ClassNotFoundException: com.sun.prism.shader.FillPgram_Color_Loader

2022-07-15 Thread Jeanette Winzenburg
Hi, after synching my master branch with upstream (has been a long while since my previous update ;), I can't use the Eclipse projects (base, controls, graphics are the only ones I'm keeping for change in Eclipse) in another project: when running any application it's throwing the stackt

Re: CFV: New OpenJFX Committer: Marius Hanl

2022-07-09 Thread Jeanette Winzenburg
Vote: YES -- Jeanette Zitat von Kevin Rushforth : I hereby nominate Marius Hanl [1] to OpenJFX Committer. Marius is an OpenJFX community member, who has contributed 22 commits [2] to OpenJFX. Votes are due by July 22, 2022 at 13:00 UTC. Only current OpenJFX Committers [3] are eligible

Re: CFV: New OpenJFX Committer: Michael Strauß

2022-07-09 Thread Jeanette Winzenburg
Vote: YES -- Jeanette Zitat von Kevin Rushforth : I hereby nominate Michael Strauß [1] to OpenJFX Committer. Michael is an OpenJFX community member, who has contributed 16 commits [2] to OpenJFX. Votes are due by July 22, 2022 at 13:00 UTC. Only current OpenJFX Committers [3] are eligi