On Mon, 25 Jul 2022 17:44:25 GMT, Andy Goryachev <[email protected]> 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?
@andy-goryachev-oracle Below are my comments for the points you raised:
1. I agree with this and upon inspecting the current code, I noticed that this
was already considered for some other bounds checking within the same method
`TableUtil.java` -> `constrainedResize(...)`.
// Check for zero. This happens when the distribution of the delta
// finishes early due to a series of "fixed" entries at the end.
// In this case, lowerBound == upperBound, for all subsequent terms.
double newSize;
if (Math.abs(totalLowerBound - totalUpperBound) < .0000001) {
newSize = lowerBound;
} else {
double f = (target - totalLowerBound) / (totalUpperBound - totalLowerBound);
newSize = Math.round(lowerBound + f * (upperBound - lowerBound));
}
Will it be ok if I create a variable named `double EPSILON=.0000001` and use
this in both the places?
2. I don't think setting TableView.CONSTRAINED_RESIZE_POLICY means to always
hide the horizontal scroll bar. The most common use case with the
CONSTRAINED_RESIZE_POLICY, is to auto stretch the columns if there is enough
space, and set to the minimum widths and show horizontal scroll bar if there is
not enough space. I included an example in this stackoverflow
[question](https://stackoverflow.com/questions/73060277/horizontal-scrollbar-is-visible-in-tableview-with-constrained-resize-policy)
and the gif show-casing the requirement (2nd gif). And the current code
already handles this very well.
-------------
PR: https://git.openjdk.org/jfx/pull/848
