Re: RFR: 8251480: TableColumnHeader: calc of cell width must respect row styling [v4]
On Wed, 30 Mar 2022 12:22:23 GMT, Robert Lichtenberger wrote: >> This fix respects a row factory, if present. >> It will put the cell that is used to measure the column width as child below >> the row. >> In that way the row's style will be used. > > Robert Lichtenberger has updated the pull request incrementally with one > additional commit since the last revision: > > 8251480: TableColumnHeader: calc of cell width must respect row styling > > Assertion removed. Looks good to me. - Marked as reviewed by mhanl (Author). PR: https://git.openjdk.java.net/jfx/pull/757
Re: Self built openjfx 17.0.2
No. What you want is the jfx17u repo. https://github.com/openjdk/jfx17u/tree/master If you look at the tags you will see 17.0.2+3 is the latest (GA) sources. -- Kevin On 3/30/2022 12:23 PM, Thiago Milczarek Sayão wrote: Hi, Does the "jfx17" branch contain the latest javafx 17.0.2 code ? https://github.com/openjdk/jfx/tree/jfx17 Also, I have built jmods of this branch with "gradlew jmodLinux" and the resulting jmods are quite bigger comparing to the released ones. I guess it's some debug symbols or something. Should I add any argument? I want the 17.0.2 with one applied extra patch. Thanks. -- Thiago
Self built openjfx 17.0.2
Hi, Does the "jfx17" branch contain the latest javafx 17.0.2 code ? https://github.com/openjdk/jfx/tree/jfx17 Also, I have built jmods of this branch with "gradlew jmodLinux" and the resulting jmods are quite bigger comparing to the released ones. I guess it's some debug symbols or something. Should I add any argument? I want the 17.0.2 with one applied extra patch. Thanks. -- Thiago
Re: RFR: 8277785: ListView scrollTo jumps to wrong location when CellHeight is changed [v3]
On Fri, 25 Mar 2022 16:26:34 GMT, Kevin Rushforth wrote: > It looks like there are some failing unit tests now. I fixed them. - PR: https://git.openjdk.java.net/jfx/pull/712
Re: RFR: 8277785: ListView scrollTo jumps to wrong location when CellHeight is changed [v4]
> When the size of a ListCell is changed and a scrollTo method is invoked > without having a layout calculation in between, the old (wrong) size is used > to calculcate the total estimate. This happens e.g. when the size is changed > in the `updateItem` method. > This PR will immediately resize the cell and sets the new value in the cache > containing the cellsizes. Johan Vos has updated the pull request incrementally with one additional commit since the last revision: Don't shift cells if we are already showing the lowest index cell. - Changes: - all: https://git.openjdk.java.net/jfx/pull/712/files - new: https://git.openjdk.java.net/jfx/pull/712/files/a931ba75..2b1b4bdc Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=712=03 - incr: https://webrevs.openjdk.java.net/?repo=jfx=712=02-03 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/712.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/712/head:pull/712 PR: https://git.openjdk.java.net/jfx/pull/712
Integrated: 8283509: Invisible menus can lead to IndexOutOfBoundsException
On Tue, 22 Mar 2022 14:42:07 GMT, Robert Lichtenberger wrote: > findSibling adapted to only use visible menus when calculating the > index. This pull request has now been integrated. Changeset: cff84e24 Author:Robert Lichtenberger Committer: Ajit Ghaisas URL: https://git.openjdk.java.net/jfx/commit/cff84e24b2de83a57163462fe275ab067034766c Stats: 34 lines in 2 files changed: 22 ins; 4 del; 8 mod 8283509: Invisible menus can lead to IndexOutOfBoundsException Reviewed-by: aghaisas - PR: https://git.openjdk.java.net/jfx/pull/759
Re: RFR: 8251480: TableColumnHeader: calc of cell width must respect row styling [v4]
On Wed, 30 Mar 2022 12:22:23 GMT, Robert Lichtenberger wrote: >> This fix respects a row factory, if present. >> It will put the cell that is used to measure the column width as child below >> the row. >> In that way the row's style will be used. > > Robert Lichtenberger has updated the pull request incrementally with one > additional commit since the last revision: > > 8251480: TableColumnHeader: calc of cell width must respect row styling > > Assertion removed. In looking at the fix, I'd like two reviewers. - PR: https://git.openjdk.java.net/jfx/pull/757
Re: RFR: 8251480: TableColumnHeader: calc of cell width must respect row styling [v4]
> This fix respects a row factory, if present. > It will put the cell that is used to measure the column width as child below > the row. > In that way the row's style will be used. Robert Lichtenberger has updated the pull request incrementally with one additional commit since the last revision: 8251480: TableColumnHeader: calc of cell width must respect row styling Assertion removed. - Changes: - all: https://git.openjdk.java.net/jfx/pull/757/files - new: https://git.openjdk.java.net/jfx/pull/757/files/960e5012..f463ab09 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=757=03 - incr: https://webrevs.openjdk.java.net/?repo=jfx=757=02-03 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jfx/pull/757.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/757/head:pull/757 PR: https://git.openjdk.java.net/jfx/pull/757
Re: RFR: 8251480: TableColumnHeader: calc of cell width must respect row styling [v2]
On Wed, 30 Mar 2022 12:10:19 GMT, Kevin Rushforth wrote: >> I found assertions in various other parts of the code (e.g. >> `javafx.scene.control.skin.VirtualFlow`) so I assumed this is ok. > > Some old code has assertions, but library code should not use them. Please > delete them from any new / modified code. Deleted the assertion. - PR: https://git.openjdk.java.net/jfx/pull/757
Re: RFR: 8251480: TableColumnHeader: calc of cell width must respect row styling [v2]
On Wed, 30 Mar 2022 10:57:38 GMT, Robert Lichtenberger wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java >> line 715: >> >>> 713: tableRow = createMeasureRow(tv, tableSkin, null); >>> 714: } >>> 715: assert tableRow.getSkin() instanceof SkinBase; >> >> Not sure if there is some convention about code asserts, maybe some else can >> tell. At least I never saw one in a PR here. > > I found assertions in various other parts of the code (e.g. > `javafx.scene.control.skin.VirtualFlow`) so I assumed this is ok. Some old code has assertions, but library code should not use them. Please delete them from any new / modified code. - PR: https://git.openjdk.java.net/jfx/pull/757
Re: RFR: 8283402: Update to gcc 11.2 on Linux
On Fri, 25 Mar 2022 12:19:10 GMT, Kevin Rushforth wrote: > This patch updates the compiler to gcc 11.2 on Linux, in order to match JDK > 17 -- see [JDK-8283057](https://bugs.openjdk.java.net/browse/JDK-8283057). > > I ran a full build and test, including media and WebKit. Given that the gcc 11.2 compiler isn't available for the latest Ubuntu LTS, 20.04, it seems worth putting this on hold for a bit. I have moved it to Draft. - PR: https://git.openjdk.java.net/jfx/pull/761
Re: RFR: 8283509: Invisible menus can lead to IndexOutOfBoundsException [v2]
On Wed, 30 Mar 2022 10:57:48 GMT, Robert Lichtenberger wrote: >> findSibling adapted to only use visible menus when calculating the >> index. > > Robert Lichtenberger has updated the pull request incrementally with one > additional commit since the last revision: > > 8251480: TableColumnHeader: calc of cell width must respect row styling > > Comment improved. Marked as reviewed by aghaisas (Reviewer). - PR: https://git.openjdk.java.net/jfx/pull/759
Re: RFR: 8283402: Update to gcc 11.2 on Linux
On Fri, 25 Mar 2022 12:19:10 GMT, Kevin Rushforth wrote: > This patch updates the compiler to gcc 11.2 on Linux, in order to match JDK > 17 -- see [JDK-8283057](https://bugs.openjdk.java.net/browse/JDK-8283057). > > I ran a full build and test, including media and WebKit. Build on Ubuntu 20.04 looked fine to me. - Marked as reviewed by arapte (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/761
Re: RFR: 8251480: TableColumnHeader: calc of cell width must respect row styling [v3]
> This fix respects a row factory, if present. > It will put the cell that is used to measure the column width as child below > the row. > In that way the row's style will be used. Robert Lichtenberger has updated the pull request incrementally with one additional commit since the last revision: 8251480: TableColumnHeader: calc of cell width must respect row styling Test readability improved. - Changes: - all: https://git.openjdk.java.net/jfx/pull/757/files - new: https://git.openjdk.java.net/jfx/pull/757/files/af582570..960e5012 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=757=02 - incr: https://webrevs.openjdk.java.net/?repo=jfx=757=01-02 Stats: 13 lines in 1 file changed: 6 ins; 6 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/757.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/757/head:pull/757 PR: https://git.openjdk.java.net/jfx/pull/757
Re: RFR: 8251480: TableColumnHeader: calc of cell width must respect row styling [v2]
On Tue, 29 Mar 2022 20:35:10 GMT, Marius Hanl wrote:
>> Robert Lichtenberger has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8251480: TableColumnHeader: calc of cell width must respect row styling
>>
>> Old behaviour restored for table rows with custom skins.
>> Unnecessary call to cell.applyCss() removed.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
> line 715:
>
>> 713: tableRow = createMeasureRow(tv, tableSkin, null);
>> 714: }
>> 715: assert tableRow.getSkin() instanceof SkinBase;
>
> Not sure if there is some convention about code asserts, maybe some else can
> tell. At least I never saw one in a PR here.
I found assertions in various other parts of the code (e.g.
`javafx.scene.control.skin.VirtualFlow`) so I assumed this is ok.
> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableColumnHeaderTest.java
> line 276:
>
>> 274: }
>> 275:
>> 276: private TableRow createSmallRow(TableView
>> tableView) {
>
> Minor: Move this method to the other row method below?
You're right. I'll move the method.
> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableColumnHeaderTest.java
> line 295:
>
>> 293: private TableRow createCustomRow(TableView
>> tableView) {
>> 294: TableRow row = new TableRow<>() {
>> 295: protected javafx.scene.control.Skin createDefaultSkin() {
>
> Very minor: `javafx.scene.control.` is not needed
You're right. I'll remove this.
-
PR: https://git.openjdk.java.net/jfx/pull/757
Re: RFR: 8283509: Invisible menus can lead to IndexOutOfBoundsException [v2]
> findSibling adapted to only use visible menus when calculating the > index. Robert Lichtenberger has updated the pull request incrementally with one additional commit since the last revision: 8251480: TableColumnHeader: calc of cell width must respect row styling Comment improved. - Changes: - all: https://git.openjdk.java.net/jfx/pull/759/files - new: https://git.openjdk.java.net/jfx/pull/759/files/2198d6bf..6d511a3f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=759=01 - incr: https://webrevs.openjdk.java.net/?repo=jfx=759=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/759.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/759/head:pull/759 PR: https://git.openjdk.java.net/jfx/pull/759
Re: RFR: 8283509: Invisible menus can lead to IndexOutOfBoundsException
On Wed, 30 Mar 2022 04:48:30 GMT, Ajit Ghaisas wrote: > > Only one minor correction needed is in comment. Right. Just pushed a correction. - PR: https://git.openjdk.java.net/jfx/pull/759
