RFR: 8089009: TableView with CONSTRAINED_RESIZE_POLICY incorrectly displays a horizontal scroll bar.

[Andy Goryachev]

On Mon, 25 Jul 2022 09:25:57 GMT, Sai Pradeep Dandem <duke at openjdk.org> wrote:

> **Issue:**
> When the `TableView` is set with built-in `CONSTRAINED_RESIZE_POLICY`, the horizontal scroll bar keeps flickering when the `TableView` width is reduced.
> 
> **Cause:**
> The table columns widths are recalculated only if the difference of the `TableView` width and the total columns width is greater than 1px. Because of this improper calculations, if the `TableView` width is reduced by 1px, the columns widths are not updated. Which results to computing for horizontal scroll bar visibility in `VirtualFlow` to improper results and let the horizontal scroll bar to be visible. 
> 
> Where as if the `TableView` width is reduced by more than 1px, the calculations are done correctly and the horizontal scroll bar visibility is turned off. Because of this behaviour, it looks like the horizontal scroll bar is flickering when the `TableView` width is reduced (let’s say by dragging).
> 
> To confirm this behaviour, please find the below example that showcases the issue:
> When the tableView width is reduced/increased by 1px, the column widths are recalculated only after every alternate 1px change. Whereas if the tableView width is reduced/increased by more than 1px (say 10px), the column widths are calculated correctly.
> 
> 
> import javafx.application.Application;
> import javafx.beans.property.SimpleStringProperty;
> import javafx.beans.property.StringProperty;
> import javafx.collections.FXCollections;
> import javafx.collections.ObservableList;
> import javafx.geometry.Insets;
> import javafx.scene.Group;
> import javafx.scene.Scene;
> import javafx.scene.control.*;
> import javafx.scene.layout.HBox;
> import javafx.scene.layout.VBox;
> import javafx.stage.Stage;
> 
> public class ConstrainedResizePolicyIssueDemo extends Application {
>     @Override
>     public void start(Stage primaryStage) throws Exception {
>         TableColumn<Person, String> fnCol = new TableColumn<>("First Name");
>         fnCol.setMinWidth(100);
>         fnCol.setCellValueFactory(param -> param.getValue().firstNameProperty());
> 
>         TableColumn<Person, String> priceCol = new TableColumn<>("Price");
>         priceCol.setMinWidth(100);
>         priceCol.setMaxWidth(150);
>         priceCol.setCellValueFactory(param -> param.getValue().priceProperty());
> 
>         TableColumn<Person, String> totalCol = new TableColumn<>("Total");
>         totalCol.setMinWidth(100);
>         totalCol.setMaxWidth(150);
>         totalCol.setCellValueFactory(param -> param.getValue().totalProperty());
> 
>         ObservableList<Person> persons = FXCollections.observableArrayList();
>         persons.add(new Person("Harry", "200.00", "210.00"));
>         persons.add(new Person("Jacob", "260.00", "260.00"));
> 
>         TableView<Person> tableView = new TableView<>();
>         tableView.getColumns().addAll(fnCol, priceCol, totalCol);
>         tableView.setItems(persons);
>         tableView.setColumnResizePolicy(TableView.CONSTRAINED_RESIZE_POLICY);
>         tableView.setMinWidth(500);
>         tableView.maxWidthProperty().bind(tableView.minWidthProperty());
> 
>         Button button1 = new Button("Reduce 1px");
>         button1.setOnAction(e -> tableView.setMinWidth(tableView.getMinWidth() - 1));
>         Button button2 = new Button("Reduce 10px");
>         button2.setOnAction(e -> tableView.setMinWidth(tableView.getMinWidth() - 10));
>         Button button3 = new Button("Reset");
>         button3.setOnAction(e -> tableView.setMinWidth(500));
>         Button button4 = new Button("Increase 1px");
>         button4.setOnAction(e -> tableView.setMinWidth(tableView.getMinWidth() + 1));
>         Button button5 = new Button("Increase 10px");
>         button5.setOnAction(e -> tableView.setMinWidth(tableView.getMinWidth() + 10));
> 
>         HBox row = new HBox(button1, button2, button3, button4, button5);
>         row.setSpacing(10);
>         TextArea output = new TextArea();
> 
>         addWidthListeners(tableView, output);
>         VBox root = new VBox(row, new Group(tableView), output);
>         root.setSpacing(10);
>         root.setPadding(new Insets(10));
> 
>         Scene scene = new Scene(root);
>         primaryStage.setScene(scene);
>         primaryStage.setTitle("Constrained Resize Policy Issue TableView");
>         primaryStage.show();
>     }
> 
>     private void addWidthListeners(TableView<Person> tableView, TextArea output) {
>         tableView.widthProperty().addListener((obs, old, val) -> {
>             String str = "Table width changed :: " + val + "\n";
>             output.setText(output.getText() + str);
>             output.positionCaret(output.getText().length());
>         });
>         tableView.getColumns().forEach(column -> {
>             column.widthProperty().addListener((obs, old, width) -> {
>                 String str = " ---> " + column.getText() + " : width changed to : " + column.getWidth()+"\n";
>                 output.setText(output.getText() + str);
>                 output.positionCaret(output.getText().length());
>             });
>         });
>     }
> 
>     class Person {
>         private StringProperty firstName = new SimpleStringProperty();
>         private StringProperty price = new SimpleStringProperty();
>         private StringProperty total = new SimpleStringProperty();
> 
>         public Person(String fn, String price, String total) {
>             setFirstName(fn);
>             setPrice(price);
>             setTotal(total);
>         }
> 
>         public StringProperty firstNameProperty() {
>             return firstName;
>         }
> 
>         public void setFirstName(String firstName) {
>             this.firstName.set(firstName);
>         }
> 
>         public StringProperty priceProperty() {
>             return price;
>         }
> 
>         public void setPrice(String price) {
>             this.price.set(price);
>         }
> 
>         public StringProperty totalProperty() {
>             return total;
>         }
> 
>         public void setTotal(String total) {
>             this.total.set(total);
>         }
>     }
> }
> 
> 
> **Fix:**
> On investigating the code, it is noticed that there is an explicit line of code to check if the difference in widths is greater than 1px. I think this should be greater than 0px. Because we need to recompute the columns widths even if the width is increased by 1px.
> 
> The part of the code that need to be updated is in TableUtil.java -> constrainedResize(..) method -> line 226
> 
> `if (Math.abs(colWidth - tableWidth) > 1) {`
> 
> If I update the value 1 to 0, the issue is fixed and the horizontal scroll bar visibility is computed correctly.

Two comments:
1. when doing a floating point arithmetic (which is imprecise) it is possible to get a value which is close, but not exactly the same as expected result.  In other words (colWidth - tableWidth) may be close to 0, but != 0.  Should we take this into account?
2. With TableView.CONSTRAINED_RESIZE_POLICY set, the application developer expects to see the horizontal scroll bar NEVER.  Is this the only place where h.s.b. gets made visible?

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

PR: https://git.openjdk.org/jfx/pull/848