On Wed, 9 Oct 2019 15:49:21 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
> 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). I wouldn't know if there are enough tests on that part. I know I added one. PR: https://git.openjdk.java.net/jfx/pull/6
