Re: RFR: 8207957: TableSkinUtils should not contain actual code implementation
On Fri, 15 Nov 2019 12:59:15 GMT, Jeanette Winzenburg
wrote:
> On Wed, 13 Nov 2019 11:26:54 GMT, Hadzic Samir wrote:
>
>> On Thu, 7 Nov 2019 12:07:55 GMT, Jeanette Winzenburg
>> wrote:
>>
>>> On Fri, 1 Nov 2019 10:59:57 GMT, Hadzic Samir wrote:
>>>
On Tue, 29 Oct 2019 13:19:27 GMT, Hadzic Samir wrote:
> On Wed, 9 Oct 2019 16:01:38 GMT, Kevin Rushforth wrote:
>
>> On Wed, 9 Oct 2019 12:26:31 GMT, Hadzic Samir
>> wrote:
>>
>>> On Mon, 7 Oct 2019 10:22:11 GMT, Jeanette Winzenburg
>>> wrote:
>>>
On Fri, 4 Oct 2019 06:13:48 GMT, Hadzic Samir
wrote:
> Allright, this is a fix for JDK-8207957
>
>
>
> Commits:
> - 969ebb51: Fixing TableColumnHeaderTest
> - 9d379619: Removing Tablecolumnbasehelper
> - 4fe020fc: Fix javadoc for TableColumnHeader
> - c422c80f: Minor modification and uni test added.
> - b2bdfb5b: Change resizeColumn to protected without static in order
> to be able to override
> - 3b9b7903: Second option for resizeColumnToFitContent in
> TableColumnHeader
> - d91c56e5: First attempt to extract resizeColumnToFitContent
>
> Changes: https://git.openjdk.java.net/jfx/pull/6/files
> Webrev: https://webrevs.openjdk.java.net/jfx/6/webrev.00
> Issue: https://bugs.openjdk.java.net/browse/JDK-8207957
> Stats: 523 lines in 4 files changed: 330 ins; 187 del; 6 mod
> Patch: https://git.openjdk.java.net/jfx/pull/6.diff
> Fetch: git fetch https://git.openjdk.java.net/jfx pull/6/head:pull/6
modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
line 600:
> 599: * expensive if the number of rows is large. Subclass can
> either call this method or override it (no need to call
> 600: * {@code super()}) to provide their custom algorithm.
> 601: *
see
https://github.com/javafxports/openjdk-jfx/pull/289#discussion_r245482213
- I think I made a suggestion (that probably needs some native
speaker's fine tuning :)
>>>
>>> Allright @kleopatra , I have indeed removed the TableColumn argument.
>>> It is clearer now that we are resizing the TableColumn of the header.
>>>
>>> I have updated the description a bit but really I'm really not good at
>>> method description so I'm open to all suggestions..
>>
>> A Draft CSR was filed here:
>> [JDK-8215554](https://bugs.openjdk.java.net/browse/JDK-8215554). It will
>> need to be updated once the API review is complete.
>
> Do you think this looks good now @kleopatra @nlisker ?
Yes I was waiting for the final 'go'. I will update it asap, thanks
Le ven. 1 nov. 2019 à 09:49, Jeanette Winzenburg
a écrit :
> *@kleopatra* commented on this pull request.
> --
>
> In
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
> :
>
> > + * Resizes this {@code TableColumnHeader}'s column to fit the
> > width of its content.
> + *
> + * @implSpec The resulting column width for this implementation is
> the maximum of the preferred width of the header
> + * cell and the preferred width of the first {@code maxRow} cells.
> + *
> + * Subclasses can either use this method or override it (without the
> need to call {@code super()}) to provide their
> + * custom implementation (such as ones that exclude the header,
> exclude {@code null} content, compute the minimum
> + * width etc.).
> + *
> + * @param maxRows the number of rows considered when resizing. If -1
> is given, all rows are considered.
> + * @since 14
>
> looks good to me :) You would have to update the crs eventually, right?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> ,
> or unsubscribe
>
> .
>
>>>
>>> As to the test, it's a bit ... confusing, will try to suggest some changes
>>> - take them with a grain of salt, though, because I'm still groping to find
>>> my path through the wilderness here :)
>>
>> I have changed the unit test according to @kleopatra review and also add two
>> more unit test regarding the
Re: RFR: 8207957: TableSkinUtils should not contain actual code implementation
On Wed, 13 Nov 2019 11:26:54 GMT, Hadzic Samir wrote:
> On Thu, 7 Nov 2019 12:07:55 GMT, Jeanette Winzenburg
> wrote:
>
>> On Fri, 1 Nov 2019 10:59:57 GMT, Hadzic Samir wrote:
>>
>>> On Tue, 29 Oct 2019 13:19:27 GMT, Hadzic Samir wrote:
>>>
On Wed, 9 Oct 2019 16:01:38 GMT, Kevin Rushforth wrote:
> On Wed, 9 Oct 2019 12:26:31 GMT, Hadzic Samir wrote:
>
>> On Mon, 7 Oct 2019 10:22:11 GMT, Jeanette Winzenburg
>> wrote:
>>
>>> On Fri, 4 Oct 2019 06:13:48 GMT, Hadzic Samir
>>> wrote:
>>>
Allright, this is a fix for JDK-8207957
Commits:
- 969ebb51: Fixing TableColumnHeaderTest
- 9d379619: Removing Tablecolumnbasehelper
- 4fe020fc: Fix javadoc for TableColumnHeader
- c422c80f: Minor modification and uni test added.
- b2bdfb5b: Change resizeColumn to protected without static in order
to be able to override
- 3b9b7903: Second option for resizeColumnToFitContent in
TableColumnHeader
- d91c56e5: First attempt to extract resizeColumnToFitContent
Changes: https://git.openjdk.java.net/jfx/pull/6/files
Webrev: https://webrevs.openjdk.java.net/jfx/6/webrev.00
Issue: https://bugs.openjdk.java.net/browse/JDK-8207957
Stats: 523 lines in 4 files changed: 330 ins; 187 del; 6 mod
Patch: https://git.openjdk.java.net/jfx/pull/6.diff
Fetch: git fetch https://git.openjdk.java.net/jfx pull/6/head:pull/6
>>>
>>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
>>> line 600:
>>>
599: * expensive if the number of rows is large. Subclass can
either call this method or override it (no need to call
600: * {@code super()}) to provide their custom algorithm.
601: *
>>>
>>> see
>>> https://github.com/javafxports/openjdk-jfx/pull/289#discussion_r245482213
>>> - I think I made a suggestion (that probably needs some native
>>> speaker's fine tuning :)
>>
>> Allright @kleopatra , I have indeed removed the TableColumn argument. It
>> is clearer now that we are resizing the TableColumn of the header.
>>
>> I have updated the description a bit but really I'm really not good at
>> method description so I'm open to all suggestions..
>
> A Draft CSR was filed here:
> [JDK-8215554](https://bugs.openjdk.java.net/browse/JDK-8215554). It will
> need to be updated once the API review is complete.
Do you think this looks good now @kleopatra @nlisker ?
>>>
>>> Yes I was waiting for the final 'go'. I will update it asap, thanks
>>>
>>> Le ven. 1 nov. 2019 à 09:49, Jeanette Winzenburg
>>> a écrit :
>>>
*@kleopatra* commented on this pull request.
--
In
modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
:
> + * Resizes this {@code TableColumnHeader}'s column to fit the width
> of its content.
+ *
+ * @implSpec The resulting column width for this implementation is
the maximum of the preferred width of the header
+ * cell and the preferred width of the first {@code maxRow} cells.
+ *
+ * Subclasses can either use this method or override it (without the
need to call {@code super()}) to provide their
+ * custom implementation (such as ones that exclude the header,
exclude {@code null} content, compute the minimum
+ * width etc.).
+ *
+ * @param maxRows the number of rows considered when resizing. If -1
is given, all rows are considered.
+ * @since 14
looks good to me :) You would have to update the crs eventually, right?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
,
or unsubscribe
.
>>
>> As to the test, it's a bit ... confusing, will try to suggest some changes -
>> take them with a grain of salt, though, because I'm still groping to find my
>> path through the wilderness here :)
>
> I have changed the unit test according to @kleopatra review and also add two
> more unit test regarding the headerContent and maxRows.
looks good to me - would approve if I could :)
PR: https://git.openjdk.java.net/jfx/pull/6
Re: RFR: 8207957: TableSkinUtils should not contain actual code implementation
On Thu, 7 Nov 2019 12:07:55 GMT, Jeanette Winzenburg
wrote:
> On Fri, 1 Nov 2019 10:59:57 GMT, Hadzic Samir wrote:
>
>> On Tue, 29 Oct 2019 13:19:27 GMT, Hadzic Samir wrote:
>>
>>> On Wed, 9 Oct 2019 16:01:38 GMT, Kevin Rushforth wrote:
>>>
On Wed, 9 Oct 2019 12:26:31 GMT, Hadzic Samir wrote:
> On Mon, 7 Oct 2019 10:22:11 GMT, Jeanette Winzenburg
> wrote:
>
>> On Fri, 4 Oct 2019 06:13:48 GMT, Hadzic Samir
>> wrote:
>>
>>> Allright, this is a fix for JDK-8207957
>>>
>>>
>>>
>>> Commits:
>>> - 969ebb51: Fixing TableColumnHeaderTest
>>> - 9d379619: Removing Tablecolumnbasehelper
>>> - 4fe020fc: Fix javadoc for TableColumnHeader
>>> - c422c80f: Minor modification and uni test added.
>>> - b2bdfb5b: Change resizeColumn to protected without static in order
>>> to be able to override
>>> - 3b9b7903: Second option for resizeColumnToFitContent in
>>> TableColumnHeader
>>> - d91c56e5: First attempt to extract resizeColumnToFitContent
>>>
>>> Changes: https://git.openjdk.java.net/jfx/pull/6/files
>>> Webrev: https://webrevs.openjdk.java.net/jfx/6/webrev.00
>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8207957
>>> Stats: 523 lines in 4 files changed: 330 ins; 187 del; 6 mod
>>> Patch: https://git.openjdk.java.net/jfx/pull/6.diff
>>> Fetch: git fetch https://git.openjdk.java.net/jfx pull/6/head:pull/6
>>
>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
>> line 600:
>>
>>> 599: * expensive if the number of rows is large. Subclass can
>>> either call this method or override it (no need to call
>>> 600: * {@code super()}) to provide their custom algorithm.
>>> 601: *
>>
>> see
>> https://github.com/javafxports/openjdk-jfx/pull/289#discussion_r245482213
>> - I think I made a suggestion (that probably needs some native
>> speaker's fine tuning :)
>
> Allright @kleopatra , I have indeed removed the TableColumn argument. It
> is clearer now that we are resizing the TableColumn of the header.
>
> I have updated the description a bit but really I'm really not good at
> method description so I'm open to all suggestions..
A Draft CSR was filed here:
[JDK-8215554](https://bugs.openjdk.java.net/browse/JDK-8215554). It will
need to be updated once the API review is complete.
>>>
>>> Do you think this looks good now @kleopatra @nlisker ?
>>
>> Yes I was waiting for the final 'go'. I will update it asap, thanks
>>
>> Le ven. 1 nov. 2019 à 09:49, Jeanette Winzenburg
>> a écrit :
>>
>>> *@kleopatra* commented on this pull request.
>>> --
>>>
>>> In
>>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
>>> :
>>>
>>> > + * Resizes this {@code TableColumnHeader}'s column to fit the width
>>> > of its content.
>>> + *
>>> + * @implSpec The resulting column width for this implementation is the
>>> maximum of the preferred width of the header
>>> + * cell and the preferred width of the first {@code maxRow} cells.
>>> + *
>>> + * Subclasses can either use this method or override it (without the
>>> need to call {@code super()}) to provide their
>>> + * custom implementation (such as ones that exclude the header,
>>> exclude {@code null} content, compute the minimum
>>> + * width etc.).
>>> + *
>>> + * @param maxRows the number of rows considered when resizing. If -1
>>> is given, all rows are considered.
>>> + * @since 14
>>>
>>> looks good to me :) You would have to update the crs eventually, right?
>>>
>>> —
>>> You are receiving this because you authored the thread.
>>> Reply to this email directly, view it on GitHub
>>> ,
>>> or unsubscribe
>>>
>>> .
>>>
>
> As to the test, it's a bit ... confusing, will try to suggest some changes -
> take them with a grain of salt, though, because I'm still groping to find my
> path through the wilderness here :)
I have changed the unit test according to @kleopatra review and also add two
more unit test regarding the headerContent and maxRows.
PR: https://git.openjdk.java.net/jfx/pull/6
Re: RFR: 8207957: TableSkinUtils should not contain actual code implementation
On Fri, 1 Nov 2019 10:59:57 GMT, Hadzic Samir wrote:
> On Tue, 29 Oct 2019 13:19:27 GMT, Hadzic Samir wrote:
>
>> On Wed, 9 Oct 2019 16:01:38 GMT, Kevin Rushforth wrote:
>>
>>> On Wed, 9 Oct 2019 12:26:31 GMT, Hadzic Samir wrote:
>>>
On Mon, 7 Oct 2019 10:22:11 GMT, Jeanette Winzenburg
wrote:
> On Fri, 4 Oct 2019 06:13:48 GMT, Hadzic Samir wrote:
>
>> Allright, this is a fix for JDK-8207957
>>
>>
>>
>> Commits:
>> - 969ebb51: Fixing TableColumnHeaderTest
>> - 9d379619: Removing Tablecolumnbasehelper
>> - 4fe020fc: Fix javadoc for TableColumnHeader
>> - c422c80f: Minor modification and uni test added.
>> - b2bdfb5b: Change resizeColumn to protected without static in order to
>> be able to override
>> - 3b9b7903: Second option for resizeColumnToFitContent in
>> TableColumnHeader
>> - d91c56e5: First attempt to extract resizeColumnToFitContent
>>
>> Changes: https://git.openjdk.java.net/jfx/pull/6/files
>> Webrev: https://webrevs.openjdk.java.net/jfx/6/webrev.00
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8207957
>> Stats: 523 lines in 4 files changed: 330 ins; 187 del; 6 mod
>> Patch: https://git.openjdk.java.net/jfx/pull/6.diff
>> Fetch: git fetch https://git.openjdk.java.net/jfx pull/6/head:pull/6
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
> line 600:
>
>> 599: * expensive if the number of rows is large. Subclass can
>> either call this method or override it (no need to call
>> 600: * {@code super()}) to provide their custom algorithm.
>> 601: *
>
> see
> https://github.com/javafxports/openjdk-jfx/pull/289#discussion_r245482213
> - I think I made a suggestion (that probably needs some native speaker's
> fine tuning :)
Allright @kleopatra , I have indeed removed the TableColumn argument. It
is clearer now that we are resizing the TableColumn of the header.
I have updated the description a bit but really I'm really not good at
method description so I'm open to all suggestions..
>>>
>>> A Draft CSR was filed here:
>>> [JDK-8215554](https://bugs.openjdk.java.net/browse/JDK-8215554). It will
>>> need to be updated once the API review is complete.
>>
>> Do you think this looks good now @kleopatra @nlisker ?
>
> Yes I was waiting for the final 'go'. I will update it asap, thanks
>
> Le ven. 1 nov. 2019 à 09:49, Jeanette Winzenburg
> a écrit :
>
>> *@kleopatra* commented on this pull request.
>> --
>>
>> In
>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
>> :
>>
>> > + * Resizes this {@code TableColumnHeader}'s column to fit the width
>> > of its content.
>> + *
>> + * @implSpec The resulting column width for this implementation is the
>> maximum of the preferred width of the header
>> + * cell and the preferred width of the first {@code maxRow} cells.
>> + *
>> + * Subclasses can either use this method or override it (without the
>> need to call {@code super()}) to provide their
>> + * custom implementation (such as ones that exclude the header, exclude
>> {@code null} content, compute the minimum
>> + * width etc.).
>> + *
>> + * @param maxRows the number of rows considered when resizing. If -1 is
>> given, all rows are considered.
>> + * @since 14
>>
>> looks good to me :) You would have to update the crs eventually, right?
>>
>> —
>> You are receiving this because you authored the thread.
>> Reply to this email directly, view it on GitHub
>> ,
>> or unsubscribe
>>
>> .
>>
As to the test, it's a bit ... confusing, will try to suggest some changes -
take them with a grain of salt, though, because I'm still groping to find my
path through the wilderness here :)
PR: https://git.openjdk.java.net/jfx/pull/6
Re: RFR: 8207957: TableSkinUtils should not contain actual code implementation
On Tue, 29 Oct 2019 13:19:27 GMT, Hadzic Samir wrote:
> On Wed, 9 Oct 2019 16:01:38 GMT, Kevin Rushforth wrote:
>
>> On Wed, 9 Oct 2019 12:26:31 GMT, Hadzic Samir wrote:
>>
>>> On Mon, 7 Oct 2019 10:22:11 GMT, Jeanette Winzenburg
>>> wrote:
>>>
On Fri, 4 Oct 2019 06:13:48 GMT, Hadzic Samir wrote:
> Allright, this is a fix for JDK-8207957
>
>
>
> Commits:
> - 969ebb51: Fixing TableColumnHeaderTest
> - 9d379619: Removing Tablecolumnbasehelper
> - 4fe020fc: Fix javadoc for TableColumnHeader
> - c422c80f: Minor modification and uni test added.
> - b2bdfb5b: Change resizeColumn to protected without static in order to
> be able to override
> - 3b9b7903: Second option for resizeColumnToFitContent in
> TableColumnHeader
> - d91c56e5: First attempt to extract resizeColumnToFitContent
>
> Changes: https://git.openjdk.java.net/jfx/pull/6/files
> Webrev: https://webrevs.openjdk.java.net/jfx/6/webrev.00
> Issue: https://bugs.openjdk.java.net/browse/JDK-8207957
> Stats: 523 lines in 4 files changed: 330 ins; 187 del; 6 mod
> Patch: https://git.openjdk.java.net/jfx/pull/6.diff
> Fetch: git fetch https://git.openjdk.java.net/jfx pull/6/head:pull/6
modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
line 600:
> 599: * expensive if the number of rows is large. Subclass can either
> call this method or override it (no need to call
> 600: * {@code super()}) to provide their custom algorithm.
> 601: *
see
https://github.com/javafxports/openjdk-jfx/pull/289#discussion_r245482213
- I think I made a suggestion (that probably needs some native speaker's
fine tuning :)
>>>
>>> Allright @kleopatra , I have indeed removed the TableColumn argument. It is
>>> clearer now that we are resizing the TableColumn of the header.
>>>
>>> I have updated the description a bit but really I'm really not good at
>>> method description so I'm open to all suggestions..
>>
>> A Draft CSR was filed here:
>> [JDK-8215554](https://bugs.openjdk.java.net/browse/JDK-8215554). It will
>> need to be updated once the API review is complete.
>
> Do you think this looks good now @kleopatra @nlisker ?
Yes I was waiting for the final 'go'. I will update it asap, thanks
Le ven. 1 nov. 2019 à 09:49, Jeanette Winzenburg
a écrit :
> *@kleopatra* commented on this pull request.
> --
>
> In
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
> :
>
> > + * Resizes this {@code TableColumnHeader}'s column to fit the width of
> > its content.
> + *
> + * @implSpec The resulting column width for this implementation is the
> maximum of the preferred width of the header
> + * cell and the preferred width of the first {@code maxRow} cells.
> + *
> + * Subclasses can either use this method or override it (without the
> need to call {@code super()}) to provide their
> + * custom implementation (such as ones that exclude the header, exclude
> {@code null} content, compute the minimum
> + * width etc.).
> + *
> + * @param maxRows the number of rows considered when resizing. If -1 is
> given, all rows are considered.
> + * @since 14
>
> looks good to me :) You would have to update the crs eventually, right?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> ,
> or unsubscribe
>
> .
>
PR: https://git.openjdk.java.net/jfx/pull/6
Re: RFR: 8207957: TableSkinUtils should not contain actual code implementation
On Wed, 9 Oct 2019 16:01:38 GMT, Kevin Rushforth wrote:
> On Wed, 9 Oct 2019 12:26:31 GMT, Hadzic Samir wrote:
>
>> On Mon, 7 Oct 2019 10:22:11 GMT, Jeanette Winzenburg
>> wrote:
>>
>>> On Fri, 4 Oct 2019 06:13:48 GMT, Hadzic Samir wrote:
>>>
Allright, this is a fix for JDK-8207957
Commits:
- 969ebb51: Fixing TableColumnHeaderTest
- 9d379619: Removing Tablecolumnbasehelper
- 4fe020fc: Fix javadoc for TableColumnHeader
- c422c80f: Minor modification and uni test added.
- b2bdfb5b: Change resizeColumn to protected without static in order to
be able to override
- 3b9b7903: Second option for resizeColumnToFitContent in
TableColumnHeader
- d91c56e5: First attempt to extract resizeColumnToFitContent
Changes: https://git.openjdk.java.net/jfx/pull/6/files
Webrev: https://webrevs.openjdk.java.net/jfx/6/webrev.00
Issue: https://bugs.openjdk.java.net/browse/JDK-8207957
Stats: 523 lines in 4 files changed: 330 ins; 187 del; 6 mod
Patch: https://git.openjdk.java.net/jfx/pull/6.diff
Fetch: git fetch https://git.openjdk.java.net/jfx pull/6/head:pull/6
>>>
>>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
>>> line 600:
>>>
599: * expensive if the number of rows is large. Subclass can either
call this method or override it (no need to call
600: * {@code super()}) to provide their custom algorithm.
601: *
>>>
>>> see
>>> https://github.com/javafxports/openjdk-jfx/pull/289#discussion_r245482213 -
>>> I think I made a suggestion (that probably needs some native speaker's fine
>>> tuning :)
>>
>> Allright @kleopatra , I have indeed removed the TableColumn argument. It is
>> clearer now that we are resizing the TableColumn of the header.
>>
>> I have updated the description a bit but really I'm really not good at
>> method description so I'm open to all suggestions..
>
> A Draft CSR was filed here:
> [JDK-8215554](https://bugs.openjdk.java.net/browse/JDK-8215554). It will need
> to be updated once the API review is complete.
Do you think this looks good now @kleopatra @nlisker ?
PR: https://git.openjdk.java.net/jfx/pull/6
Re: RFR: 8207957: TableSkinUtils should not contain actual code implementation
On Wed, 9 Oct 2019 12:26:31 GMT, Hadzic Samir wrote:
> On Mon, 7 Oct 2019 10:22:11 GMT, Jeanette Winzenburg
> wrote:
>
>> On Fri, 4 Oct 2019 06:13:48 GMT, Hadzic Samir wrote:
>>
>>> Allright, this is a fix for JDK-8207957
>>>
>>>
>>>
>>> Commits:
>>> - 969ebb51: Fixing TableColumnHeaderTest
>>> - 9d379619: Removing Tablecolumnbasehelper
>>> - 4fe020fc: Fix javadoc for TableColumnHeader
>>> - c422c80f: Minor modification and uni test added.
>>> - b2bdfb5b: Change resizeColumn to protected without static in order to be
>>> able to override
>>> - 3b9b7903: Second option for resizeColumnToFitContent in TableColumnHeader
>>> - d91c56e5: First attempt to extract resizeColumnToFitContent
>>>
>>> Changes: https://git.openjdk.java.net/jfx/pull/6/files
>>> Webrev: https://webrevs.openjdk.java.net/jfx/6/webrev.00
>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8207957
>>> Stats: 523 lines in 4 files changed: 330 ins; 187 del; 6 mod
>>> Patch: https://git.openjdk.java.net/jfx/pull/6.diff
>>> Fetch: git fetch https://git.openjdk.java.net/jfx pull/6/head:pull/6
>>
>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
>> line 600:
>>
>>> 599: * expensive if the number of rows is large. Subclass can either
>>> call this method or override it (no need to call
>>> 600: * {@code super()}) to provide their custom algorithm.
>>> 601: *
>>
>> see
>> https://github.com/javafxports/openjdk-jfx/pull/289#discussion_r245482213 -
>> I think I made a suggestion (that probably needs some native speaker's fine
>> tuning :)
>
> Allright @kleopatra , I have indeed removed the TableColumn argument. It is
> clearer now that we are resizing the TableColumn of the header.
>
> I have updated the description a bit but really I'm really not good at method
> description so I'm open to all suggestions..
A Draft CSR was filed here:
[JDK-8215554](https://bugs.openjdk.java.net/browse/JDK-8215554). It will need
to be updated once the API review is complete.
PR: https://git.openjdk.java.net/jfx/pull/6
Re: RFR: 8207957: TableSkinUtils should not contain actual code implementation
On Mon, 7 Oct 2019 10:22:11 GMT, Jeanette Winzenburg
wrote:
> On Fri, 4 Oct 2019 06:13:48 GMT, Hadzic Samir wrote:
>
>> Allright, this is a fix for JDK-8207957
>>
>>
>>
>> Commits:
>> - 969ebb51: Fixing TableColumnHeaderTest
>> - 9d379619: Removing Tablecolumnbasehelper
>> - 4fe020fc: Fix javadoc for TableColumnHeader
>> - c422c80f: Minor modification and uni test added.
>> - b2bdfb5b: Change resizeColumn to protected without static in order to be
>> able to override
>> - 3b9b7903: Second option for resizeColumnToFitContent in TableColumnHeader
>> - d91c56e5: First attempt to extract resizeColumnToFitContent
>>
>> Changes: https://git.openjdk.java.net/jfx/pull/6/files
>> Webrev: https://webrevs.openjdk.java.net/jfx/6/webrev.00
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8207957
>> Stats: 523 lines in 4 files changed: 330 ins; 187 del; 6 mod
>> Patch: https://git.openjdk.java.net/jfx/pull/6.diff
>> Fetch: git fetch https://git.openjdk.java.net/jfx pull/6/head:pull/6
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
> line 600:
>
>> 599: * expensive if the number of rows is large. Subclass can either
>> call this method or override it (no need to call
>> 600: * {@code super()}) to provide their custom algorithm.
>> 601: *
>
> see https://github.com/javafxports/openjdk-jfx/pull/289#discussion_r245482213
> - I think I made a suggestion (that probably needs some native speaker's fine
> tuning :)
Allright @kleopatra , I have indeed removed the TableColumn argument. It is
clearer now that we are resizing the TableColumn of the header.
I have updated the description a bit but really I'm really not good at method
description so I'm open to all suggestions..
PR: https://git.openjdk.java.net/jfx/pull/6
Re: RFR: 8207957: TableSkinUtils should not contain actual code implementation
On Fri, 4 Oct 2019 06:13:48 GMT, Hadzic Samir wrote:
> Allright, this is a fix for JDK-8207957
>
>
>
> Commits:
> - 969ebb51: Fixing TableColumnHeaderTest
> - 9d379619: Removing Tablecolumnbasehelper
> - 4fe020fc: Fix javadoc for TableColumnHeader
> - c422c80f: Minor modification and uni test added.
> - b2bdfb5b: Change resizeColumn to protected without static in order to be
> able to override
> - 3b9b7903: Second option for resizeColumnToFitContent in TableColumnHeader
> - d91c56e5: First attempt to extract resizeColumnToFitContent
>
> Changes: https://git.openjdk.java.net/jfx/pull/6/files
> Webrev: https://webrevs.openjdk.java.net/jfx/6/webrev.00
> Issue: https://bugs.openjdk.java.net/browse/JDK-8207957
> Stats: 523 lines in 4 files changed: 330 ins; 187 del; 6 mod
> Patch: https://git.openjdk.java.net/jfx/pull/6.diff
> Fetch: git fetch https://git.openjdk.java.net/jfx pull/6/head:pull/6
modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
line 600:
> 599: * expensive if the number of rows is large. Subclass can either
> call this method or override it (no need to call
> 600: * {@code super()}) to provide their custom algorithm.
> 601: *
see https://github.com/javafxports/openjdk-jfx/pull/289#discussion_r245482213 -
I think I made a suggestion (that probably needs some native speaker's fine
tuning :)
PR: https://git.openjdk.java.net/jfx/pull/6
Re: RFR: 8207957: TableSkinUtils should not contain actual code implementation
On Fri, 4 Oct 2019 06:13:48 GMT, Hadzic Samir wrote: > Allright, this is a fix for JDK-8207957 > > > > Commits: > - 969ebb51: Fixing TableColumnHeaderTest > - 9d379619: Removing Tablecolumnbasehelper > - 4fe020fc: Fix javadoc for TableColumnHeader > - c422c80f: Minor modification and uni test added. > - b2bdfb5b: Change resizeColumn to protected without static in order to be > able to override > - 3b9b7903: Second option for resizeColumnToFitContent in TableColumnHeader > - d91c56e5: First attempt to extract resizeColumnToFitContent > > Changes: https://git.openjdk.java.net/jfx/pull/6/files > Webrev: https://webrevs.openjdk.java.net/jfx/6/webrev.00 > Issue: https://bugs.openjdk.java.net/browse/JDK-8207957 > Stats: 523 lines in 4 files changed: 330 ins; 187 del; 6 mod > Patch: https://git.openjdk.java.net/jfx/pull/6.diff > Fetch: git fetch https://git.openjdk.java.net/jfx pull/6/head:pull/6 modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java line 608: > 607: if (!tc.isResizable()) return; > 608: > 609: Object control = this.getTableSkin().getSkinnable(); see https://github.com/javafxports/openjdk-jfx/pull/289#discussion_r245482258 - this api still looks unhealthy .. PR: https://git.openjdk.java.net/jfx/pull/6
Re: RFR: 8207957: TableSkinUtils should not contain actual code implementation
On Fri, 4 Oct 2019 06:13:48 GMT, Hadzic Samir wrote: > Allright, this is a fix for JDK-8207957 > > > > Commits: > - 969ebb51: Fixing TableColumnHeaderTest > - 9d379619: Removing Tablecolumnbasehelper > - 4fe020fc: Fix javadoc for TableColumnHeader > - c422c80f: Minor modification and uni test added. > - b2bdfb5b: Change resizeColumn to protected without static in order to be > able to override > - 3b9b7903: Second option for resizeColumnToFitContent in TableColumnHeader > - d91c56e5: First attempt to extract resizeColumnToFitContent > > Changes: https://git.openjdk.java.net/jfx/pull/6/files > Webrev: https://webrevs.openjdk.java.net/jfx/6/webrev.00 > Issue: https://bugs.openjdk.java.net/browse/JDK-8207957 > Stats: 523 lines in 4 files changed: 330 ins; 187 del; 6 mod > Patch: https://git.openjdk.java.net/jfx/pull/6.diff > Fetch: git fetch https://git.openjdk.java.net/jfx pull/6/head:pull/6 JBS bug: [JDK-8207957](https://bugs.openjdk.java.net/browse/JDK-8207957) Review comments from preliminary review: [javafxports/openjdk-jfx/pull/289](https://github.com/javafxports/openjdk-jfx/pull/289) PR: https://git.openjdk.java.net/jfx/pull/6
