Bug ID: JDK-8294398 Additional constrained resize policies for Tree/TableView

JDK-8294398 : Additional constrained resize policies for Tree/TableView

Type: CSR
Component: javafx
Sub-Component: controls

Priority: P4
Status: Closed
Resolution: Approved
Fix Versions: openjfx20

Submitted: 2022-09-26
Updated: 2022-12-29
Resolved: 2022-12-29

Related Reports

CSR :

JDK-8293119 - Additional constrained resize policies for Tree/TableView

Description

Summary
-------

- Adding several new constrained resize policies, similar to JTable
- Replacing buggy Tree/TableView.CONSTRAINED_RESIZE_POLICY with a new implementation
- Adding new methods to make it possible to create user-defined Tree/TableView column resize policies

Problem
-------

The current Tree/TableView.CONSTRAINED_RESIZE_POLICY has many, many issues, see JDK-8292810.  One such issue is a complete inability to deal with over-constrained Tree/TableView.

In addition to that, any developer who wants to fix the situation by providing their own implementation (as suggested by existing setColumnResizePolicy() method) is simply unable to do so because no public APIs exist that allow for actually setting the column widths, or to obtain the target width.



Solution
--------

First, public APIs are added to `ResizeFeaturesBase` class to make sure the application developers *can* create a custom resize policy.  These changes include:

- made ResizeFeaturesBase class abstract
- added `abstract getContentWidth()`
- added `setColumnWidth(TableColumnBase, double);`
- added `abstract getTableControl()` method to return the Tree/TableView control

Added a new abstract ConstrainedColumnResizeBase class which serves as a marker class to tell the skin to disable the horizontal scroll bar.

Added a number of additional Tree/TableView column resize policies 

- CONSTRAINED_RESIZE_POLICY_ALL_COLUMNS
- CONSTRAINED_RESIZE_POLICY_LAST_COLUMN
- CONSTRAINED_RESIZE_POLICY_NEXT_COLUMN
- CONSTRAINED_RESIZE_POLICY_SUBSEQUENT_COLUMNS
- CONSTRAINED_RESIZE_POLICY_FLEX_NEXT_COLUMN
- CONSTRAINED_RESIZE_POLICY_FLEX_LAST_COLUMN

And finally, replaced the legacy CONSTRAINED_RESIZE_POLICY with a new *bugless* implementation CONSTRAINED_RESIZE_POLICY_FLEX_LAST_COLUMN.


Specification
-------------

```
	modules/javafx.controls/src/main/java/javafx/scene/control/ConstrainedColumnResizeBase.java
	
	+package javafx.scene.control;
	+
	+/**
	+ * Base class for a constrained column resize policy.
	+ * Setting any policy that extends this class on a Tree/TableView results in
	+ * disabling of its horizontal scroll bar.
	+ *
	+ * @see TableView#columnResizePolicyProperty
	+ * @see TreeTableView#columnResizePolicyProperty
	+ *
	+ * @since 20
	+ */
	+public abstract class ConstrainedColumnResizeBase {
	+    /**
	+     * Constructor for subclasses to call.
	+     */
	+    public ConstrainedColumnResizeBase() {
	+    }
	+
	+    @Override
	+    public String toString() {
	+        // name of a pseudo-style set on a Tree/TableView when a constrained resize policy is in effect
	+        return "constrained-resize";
	+    }
	+}
	
	
	modules/javafx.controls/src/main/java/javafx/scene/control/ResizeFeaturesBase.java
	 
	 /**
	- * An immutable wrapper class for use by the column resize policies offered by
	+ * A wrapper class for use by the column resize policies offered by
	  * controls such as {@link TableView} and {@link TreeTableView}.
	  * @since JavaFX 8.0
	  */
	-public class ResizeFeaturesBase<S> {
	+public abstract class ResizeFeaturesBase<S> {
	
	
	   /**
	+   * Returns the width of the area available for columns.
	+   *
	+   * @return the width availabe for columns
	+   *
	+   * @since 20
	+   */
	+  public abstract double getContentWidth();
	+
	+  /**
	+   * Returns the associated TreeView or TreeTableView control.
	+   *
	+   * @return the control in which the resize is occurring
	+   *
	+   * @since 20
	+   */
	+  public abstract Control getTableControl();
	
	+  /**
	+   * Sets the column width during the resizing pass.
	+   *
	+   * @param col column being changed
	+   * @param width desired column width
	+   *
	+   * @since 20
	+   */
	+  public void setColumnWidth(TableColumnBase<S,?> col, double width) {
	+      col.doSetWidth(width);
	+  }
	
	
	b/modules/javafx.controls/src/main/java/javafx/scene/control/TableView.java
	
	+     * A resize policy that adjusts other columns in order to fit the table width.
	+     * During UI adjustment, proportionately resizes all columns to preserve the total width.
	+     * <p>
	+     * When column constraints make it impossible to fit all the columns into the allowed area,
	+     * the columns are either clipped, or an empty space appears.  This policy disables the horizontal
	+     * scroll bar.
	+     *
	+     * @since 20
	+     */
	+    public static final Callback<TableView.ResizeFeatures, Boolean> CONSTRAINED_RESIZE_POLICY_ALL_COLUMNS =
	+        ConstrainedColumnResize.forTable(ConstrainedColumnResize.ResizeMode.AUTO_RESIZE_ALL_COLUMNS);
	+
	+    /**
	+     * A resize policy that adjusts the last column in order to fit the table width.
	+     * During UI adjustment, resizes the last column only to preserve the total width.
	+     * <p>
	+     * When column constraints make it impossible to fit all the columns into the allowed area,
	+     * the columns are either clipped, or an empty space appears.  This policy disables the horizontal
	+     * scroll bar.
	+     *
	+     * @since 20
	+     */
	+    public static final Callback<TableView.ResizeFeatures, Boolean> CONSTRAINED_RESIZE_POLICY_LAST_COLUMN =
	+        ConstrainedColumnResize.forTable(ConstrainedColumnResize.ResizeMode.AUTO_RESIZE_LAST_COLUMN);
	+
	+    /**
	+     * A resize policy that adjusts the next column in order to fit the table width.
	+     * During UI adjustment, resizes the next column the opposite way.
	+     * <p>
	+     * When column constraints make it impossible to fit all the columns into the allowed area,
	+     * the columns are either clipped, or an empty space appears.  This policy disables the horizontal
	+     * scroll bar.
	+     *
	+     * @since 20
	+     */
	+    public static final Callback<TableView.ResizeFeatures, Boolean> CONSTRAINED_RESIZE_POLICY_NEXT_COLUMN =
	+        ConstrainedColumnResize.forTable(ConstrainedColumnResize.ResizeMode.AUTO_RESIZE_NEXT_COLUMN);
	+
	+    /**
	+     * A resize policy that adjusts subsequent columns in order to fit the table width.
	+     * During UI adjustment, proportionally resizes subsequent columns to preserve the total width.
	+     * <p>
	+     * When column constraints make it impossible to fit all the columns into the allowed area,
	+     * the columns are either clipped, or an empty space appears.  This policy disables the horizontal
	+     * scroll bar.
	+     *
	+     * @since 20
	+     */
	+    public static final Callback<TableView.ResizeFeatures, Boolean> CONSTRAINED_RESIZE_POLICY_SUBSEQUENT_COLUMNS =
	+        ConstrainedColumnResize.forTable(ConstrainedColumnResize.ResizeMode.AUTO_RESIZE_SUBSEQUENT_COLUMNS);
	+
	+    /**
	+     * A resize policy that adjusts columns, starting with the next one, in order to fit the table width.
	+     * During UI adjustment, resizes the next column to preserve the total width.  When the next column
	+     * cannot be further resized due to a constraint, the following column gets resized, and so on.
	+     * <p>
	+     * When column constraints make it impossible to fit all the columns into the allowed area,
	+     * the columns are either clipped, or an empty space appears.  This policy disables the horizontal
	+     * scroll bar.
	+     *
	+     * @since 20
	+     */
	+    public static final Callback<TableView.ResizeFeatures, Boolean> CONSTRAINED_RESIZE_POLICY_FLEX_NEXT_COLUMN =
	+        ConstrainedColumnResize.forTable(ConstrainedColumnResize.ResizeMode.AUTO_RESIZE_FLEX_HEAD);
	+
	+    /**
	+     * A resize policy that adjusts columns, starting with the last one, in order to fit the table width.
	+     * During UI adjustment, resizes the last column to preserve the total width.  When the last column
	+     * cannot be further resized due to a constraint, the column preceding the last one gets resized, and so on.
	+     * <p>
	+     * When column constraints make it impossible to fit all the columns into the allowed area,
	+     * the columns are either clipped, or an empty space appears.  This policy disables the horizontal
	+     * scroll bar.
	+     *
	+     * @since 20
	+     */
	+    public static final Callback<TableView.ResizeFeatures, Boolean> CONSTRAINED_RESIZE_POLICY_FLEX_LAST_COLUMN =
	+        ConstrainedColumnResize.forTable(ConstrainedColumnResize.ResizeMode.AUTO_RESIZE_FLEX_TAIL);
	+
	+    /**
	      * <p>Simple policy that ensures the width of all visible leaf columns in
	...
	+     *
	+     * @deprecated Use {@link #CONSTRAINED_RESIZE_POLICY_FLEX_LAST_COLUMN} instead.
	      */
	-    public static final Callback<ResizeFeatures, Boolean> CONSTRAINED_RESIZE_POLICY = new Callback<>() {
	...
	+    @Deprecated(since="20")
	+    public static final Callback<ResizeFeatures, Boolean> CONSTRAINED_RESIZE_POLICY =
	+        CONSTRAINED_RESIZE_POLICY_FLEX_LAST_COLUMN;
	 
	
	modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java
	
	+     * A resize policy that adjusts other columns in order to fit the tree table width.
	+     * During UI adjustment, proportionately resizes all columns to preserve the total width.
	+     * <p>
	+     * When column constraints make it impossible to fit all the columns into the allowed area,
	+     * the columns are either clipped, or an empty space appears.  This policy disables the horizontal
	+     * scroll bar.
	+     *
	+     * @since 20
	+     */
	+    public static final Callback<TreeTableView.ResizeFeatures, Boolean> CONSTRAINED_RESIZE_POLICY_ALL_COLUMNS =
	+        ConstrainedColumnResize.forTreeTable(ConstrainedColumnResize.ResizeMode.AUTO_RESIZE_ALL_COLUMNS);
	+
	+    /**
	+     * A resize policy that adjusts the last column in order to fit the tree table width.
	+     * During UI adjustment, resizes the last column only to preserve the total width.
	+     * <p>
	+     * When column constraints make it impossible to fit all the columns into the allowed area,
	+     * the columns are either clipped, or an empty space appears.  This policy disables the horizontal
	+     * scroll bar.
	+     *
	+     * @since 20
	+     */
	+    public static final Callback<TreeTableView.ResizeFeatures, Boolean> CONSTRAINED_RESIZE_POLICY_LAST_COLUMN =
	+        ConstrainedColumnResize.forTreeTable(ConstrainedColumnResize.ResizeMode.AUTO_RESIZE_LAST_COLUMN);
	+
	+    /**
	+     * A resize policy that adjusts the next column in order to fit the tree table width.
	+     * During UI adjustment, resizes the next column the opposite way.
	+     * <p>
	+     * When column constraints make it impossible to fit all the columns into the allowed area,
	+     * the columns are either clipped, or an empty space appears.  This policy disables the horizontal
	+     * scroll bar.
	+     *
	+     * @since 20
	+     */
	+    public static final Callback<TreeTableView.ResizeFeatures, Boolean> CONSTRAINED_RESIZE_POLICY_NEXT_COLUMN =
	+        ConstrainedColumnResize.forTreeTable(ConstrainedColumnResize.ResizeMode.AUTO_RESIZE_NEXT_COLUMN);
	+
	+    /**
	+     * A resize policy that adjusts subsequent columns in order to fit the tree table width.
	+     * During UI adjustment, proportionally resizes subsequent columns to preserve the total width.
	+     * <p>
	+     * When column constraints make it impossible to fit all the columns into the allowed area,
	+     * the columns are either clipped, or an empty space appears.  This policy disables the horizontal
	+     * scroll bar.
	+     *
	+     * @since 20
	+     */
	+    public static final Callback<TreeTableView.ResizeFeatures, Boolean> CONSTRAINED_RESIZE_POLICY_SUBSEQUENT_COLUMNS =
	+        ConstrainedColumnResize.forTreeTable(ConstrainedColumnResize.ResizeMode.AUTO_RESIZE_SUBSEQUENT_COLUMNS);
	+
	+    /**
	+     * A resize policy that adjusts columns, starting with the next one, in order to fit the tree table width.
	+     * During UI adjustment, resizes the next column to preserve the total width.  When the next column
	+     * cannot be further resized due to a constraint, the following column gets resized, and so on.
	+     * <p>
	+     * When column constraints make it impossible to fit all the columns into the allowed area,
	+     * the columns are either clipped, or an empty space appears.  This policy disables the horizontal
	+     * scroll bar.
	+     *
	+     * @since 20
	+     */
	+    public static final Callback<TreeTableView.ResizeFeatures, Boolean> CONSTRAINED_RESIZE_POLICY_FLEX_NEXT_COLUMN =
	+        ConstrainedColumnResize.forTreeTable(ConstrainedColumnResize.ResizeMode.AUTO_RESIZE_FLEX_HEAD);
	+
	+    /**
	+     * A resize policy that adjusts columns, starting with the last one, in order to fit the table width.
	+     * During UI adjustment, resizes the last column to preserve the total width.  When the last column
	+     * cannot be further resized due to a constraint, the column preceding the last one gets resized, and so on.
	+     * <p>
	+     * When column constraints make it impossible to fit all the columns into the allowed area,
	+     * the columns are either clipped, or an empty space appears.  This policy disables the horizontal
	+     * scroll bar.
	+     *
	+     * @since 20
	+     */
	+    public static final Callback<TreeTableView.ResizeFeatures, Boolean> CONSTRAINED_RESIZE_POLICY_FLEX_LAST_COLUMN =
	+        ConstrainedColumnResize.forTreeTable(ConstrainedColumnResize.ResizeMode.AUTO_RESIZE_FLEX_TAIL);
	+
	+    /**
	      * <p>Simple policy that ensures the width of all visible leaf columns in
	...
	+     *
	+     * @deprecated Use {@link #CONSTRAINED_RESIZE_POLICY_FLEX_LAST_COLUMN} instead.
	      */
	+    @Deprecated(since="20")
	     public static final Callback<TreeTableView.ResizeFeatures, Boolean> CONSTRAINED_RESIZE_POLICY =
	-            new Callback<>() {
    ...	
	+        CONSTRAINED_RESIZE_POLICY_FLEX_LAST_COLUMN;
```

Comments

Moving to Approved.
29-12-2022
Looks great.
20-12-2022
Thanks, this looks much better. You could also eliminate the former implementation of the now-deprecated `CONSTRAINED_RESIZE_POLICY` (from both TableView and TreeTableVIew), by removing the following: ``` - - private boolean isFirstRun = true; - - @Override public String toString() { - return "constrained-resize"; - } - - @Override public Boolean call(ResizeFeatures prop) { - TableView<?> table = prop.getTable(); - List<? extends TableColumnBase<?,?>> visibleLeafColumns = table.getVisibleLeafColumns(); - Boolean result = TableUtil.constrainedResize(prop, - isFirstRun, - table.contentWidth, - visibleLeafColumns); - isFirstRun = ! isFirstRun ? false : ! result; - return result; - } - }; ```
20-12-2022
indenting code block worked, thank you for your suggestions!
20-12-2022
Also, the first sentence of the Compatibility Risk field needs to be updated slightly now that `ResizeFeaturesBase` is an abstract class.
20-12-2022
The changes to the API docs look like what I would expect. Can you trim the diffs to exclude the implementation, import statements, copyrights, and so forth, so it will be easier to see just the spec changes? Also, it would be easier to read and compare as a text file (rather than a Microsoft word doc). Better yet, I figured out what will fix the diffs so that they can appear inline in the Specification section of the Description. In addition to enclosing in a pair of triple back-quotes, if you indent all lines of the the diffs by 4 spaces (i.e., everything between the starting and ending triple back-quote), it will show up correctly. I recall having had to do this in other CSRs.
20-12-2022
diffs updated
19-12-2022
I'll re-review after the two changes identified in my two previous comments are made.
19-12-2022
One last API change was identified during code review. There is a missing default constructor for the new `ConstrainedColumnResizeBase` class.
19-12-2022
After discussing it, we propose to do option 2: change the class to be abstract, and then make the two newly-added methods abstract. We don't expect that any existing application could usefully have subclassed ResizeFeatureBase, nor constructed a concrete class. Andy came up with a use-case for subclassing ResizeFeatureBase: with the addition of those two methods, it is now possible to create a custom resize policy, and subclassing ResizeFeatureBase could aid in testing. As a result, we don't plan to make it a sealed class, nor do we plan to deprecate the constructor for removal (which we had planned to do separately).
13-12-2022
[~kcr], the worst-case outcomes of the two approaches are similar, but there are variations as the previous comments suggest. For approach 1, the "default method" approach the failure would only occur if the unsupported method was actually called. For approach 2, the failure might occur when objects of the existing type were attempted to be created (i.e. much earlier). If actual subclasses of ResizeFeatureBase outside of FX's maintenance domain are not expected, then I think 2) is a cleaner solution (perhaps made even stricter by sealing the type if possible). However, if the possibility of that incompatibility is too large to accept, 2 allows more cases to keep working. HTH Moving to Provisional. At your option, happy to have further discussion about alternatives -- I would accept either alternative being Finalized after your subsequent analysis.
13-12-2022
I might add that ResizeFeaturesBase is created by a Tree/TableView and passed on to a resize policy implementation. There is even a call to deprecate its public constructor in JDK-8294469. While I understand the desire to keep things away from those pesky application developers, I would rather keep the public constructor (for testing purposes, for example), and made ResizeFeatureBase abstract for the reasons [~kcr] outlined earlier. I think there is a little risk of any compatibility problems since it highly unlikely that anyone extended this class except for the two child classes inside javafx.
10-12-2022
[~darcy] Given that we have made ResizeFeaturesBase effectively abstract, what is your view on which incompatible change is better: 1. Leave it concrete, but throw an exception at runtime if the two new methods aren't overridden 2. Actually make it abstract, which will enforce the requirement to override the methods at compile time I initially suggested that Andy pursue option 1, which is what is currently specified, but given that they seem like equally incompatible choices, I wonder if 2 might be better, since it is more explicit?
10-12-2022
One more comment on the Solutions section: > made ResizeFeaturesBase class abstract Actually, the current version leaves it as a concrete class, but adds two new methods that, if not overridden, will throw a UOE. Given that the likelihood of any app actually using it directly is low, I doubt that this matters.
09-12-2022
This looks good to me. There might be some minor wording changes, but I don't anticipate anything that would impact the API.
09-12-2022
Please see attachment - I cannot make JIRA to display the code blocks correctly.
09-12-2022