On Wed, 9 Oct 2019 12:25:26 GMT, Hadzic Samir <shad...@openjdk.org> wrote:

> The pull request has been updated with additional changes.
> 
> ----------------
> 
> Added commits:
>  - e846e51c: Remove TableColumn argument for resizeColumnToFitContent for 
> clarification on TableColumnHeader
> 
> Changes:
>   - all: https://git.openjdk.java.net/jfx/pull/6/files
>   - new: https://git.openjdk.java.net/jfx/pull/6/files/969ebb51..e846e51c
> 
> Webrevs:
>  - full: https://webrevs.openjdk.java.net/jfx/6/webrev.01
>  - incr: https://webrevs.openjdk.java.net/jfx/6/webrev.00-01
> 
>   Issue: https://bugs.openjdk.java.net/browse/JDK-8207957
>   Stats: 27 lines in 3 files changed: 13 ins; 1 del; 13 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/NestedTableColumnHeader.java
 line 170:

> 169:             TableColumnHeader columnHeader = 
> tableHeader.getColumnHeaderFor(column);
> 170:             if(columnHeader != null){
> 171:                 columnHeader.resizeColumnToFitContent(-1);

Space after `if` and before `{`.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
 line 608:

> 607:     protected void resizeColumnToFitContent(int maxRows) {
> 608:         TableColumnBase<?, ?> tc = getTableColumn();
> 609:         if (!tc.isResizable()) return;

Here's my suggestion (mind the max character length per line):

Resizes this {@code TableColumnHeader}'s column to fit the width of its 
content. The resulting column width is the maximum of the preferred width of 
the header cell and the preferred width of the first {@code maxRow} cells.
<p>
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.).

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
 line 618:

> 617:     }
> 618: 
> 619:     private <T,S> void resizeColumnToFitContent(TableView<T> tv, 
> TableColumn<T, S> tc, TableViewSkinBase tableSkin, int maxRows) {

I think we put a space when casting, like `(TableView) control`. Not sure if it 
is a rule.

Since you are adding new API you will need to file a CSR once the API review id 
done.

----------------

Disapproved by nlisker (Committer).

PR: https://git.openjdk.java.net/jfx/pull/6

Reply via email to