Added option to control the ScrollBar policy of each scroll bar

[JordanMartinez]

Note: these warnings still appear

Flowless/src/main/java/org/fxmisc/flowless/VirtualizedScrollPane.java:32: warning - Tag @see: can't find setHbarPolicy(ScrollBarPolicy) in org.fxmisc.flowless.VirtualizedScrollPane Flowless/src/main/java/org/fxmisc/flowless/VirtualizedScrollPane.java:38: warning - Tag @see: can't find setVbarPolicy(ScrollBarPolicy) in org.fxmisc.flowless.VirtualizedScrollPane Flowless/src/main/java/org/fxmisc/flowless/VirtualizedScrollPane.java:32: warning - Tag @see: can't find setHbarPolicy(ScrollBarPolicy) in org.fxmisc.flowless.VirtualizedScrollPane Flowless/src/main/java/org/fxmisc/flowless/VirtualizedScrollPane.java:38: warning - Tag @see: can't find setVbarPolicy(ScrollBarPolicy) in org.fxmisc.flowless.VirtualizedScrollPane

[JordanMartinez]

So, it seems like the javadoc warning is caused by a bug in the javadoc. I looked around on the web and couldn't find a good reason for why it was occurring.

I did notice that the bug is reproducible in the following code pattern:

public Test {
    private final Var<BarPolicy> verticalPolicy;
    public final BarPolicy getVerticalPolicy() { return verticalPolicy.getValue(); }
    public final void setVerticalPolicy(BarPolicy value) { verticalPolicy.setValue(value); }
    public final Var<BarPolicy> verticalPolicyProperty() { return verticalPolicy; }

    public Test(BarPolicy policy) {
        this.verticalPolicy = Var.newSimpleVar(policy);
    }
}

public static enum BarPolicy {
    FAKE
}

I'll see if any difference arises when I remove the ReactFX dependency and just use a SimpleObjectProperty

[Groostav]

So this is tangentially related, but since you're working in this area I'll ask you if you can add a feature for me: Is there a way to get a VirtualFlow for use with horizontal scrolling only?

If I want a virtual flow to control horizontal scrolling, but delegate vertical scrolling to its acenstors (--in my case actually a regular old ScrollPane), how could I do that?

Specifically I feel like this could be expressed relatively easily with:

// horizontallyScrollable {get; set; property}

-public void scrollX(double deltaX) {
+public boolean scrollX(double deltaX) {
-  content.scrollX(deltaX);
+  return if(isHorizontallyScrollable()) { context.scrollX(deltaX); }
}

//similar for Y

this.addEventHandler(ScrollEvent.SCROLL, se -> {
-  scrollX(-se.getDeltaX());
-  scrollY(-se.getDeltaY());
-  se.consume();
+  boolean changed = scrollX(-se.getDeltaX());
+  changed |= scrollY(-se.getDeltaY());
+  if(changed) { se.consume(); }
});

can I ask you to make something like this part of that PR?

edit: actually that's pretty silly, why not just have the event handler compute whether or not it should consume the scroll event based on whether or not the component has anything to scroll?

[JordanMartinez]

actually that's pretty silly, why not just have the event handler compute whether or not it should consume the scroll event based on whether or not the component has anything to scroll?

:smile: Yeah, that seems like a better approach.

[JordanMartinez]

Speaking of which, @TomasMikula is this PR acceptable if I cleaned it up and removed some of the unnecessary intermediate commits?

Also, I think it would be better to have ScrollPane.ScrollBarPolicy as the type to use for the scroll bars since a class displaying one or more of VirtualizedScrollPane and ScrollPane could use a single import, but I can't figure out how to stop that tag warning from appearing. So, would you rather have the warnings but single import? Or multiple imports but no warnings?

[TomasMikula]

The corner issue should be handled separately (has nothing to do with scroll bar policy).

I would reuse ScrollPane.ScrollBarPolicy.

The change seems somewhat verbose. Why isn't it sufficient to define shouldDisplayHorizontal/shouldDisplayVertical as you do, bind hbar's/vbar's visibleProperty() to those (as you do) and leave the rest as is? What is achieved by changes in layoutChildren()?

[JordanMartinez]

The corner issue should be handled separately (has nothing to do with scroll bar policy).

True.

I would reuse ScrollPane.ScrollBarPolicy.

K.

The change seems somewhat verbose. Why isn't it sufficient to define shouldDisplayHorizontal/shouldDisplayVertical as you do, bind hbar's/vbar's visibleProperty() to those (as you do) and leave the rest as is? What is achieved by changes in layoutChildren()?

After checking... Nothing, actually. Since this was done long ago, I can't recall everything that was going through my mind. However, I do remember thinking it would be better to have shouldDisplayHorizontal/Vertical combined into shouldDisplayBoth so that only one InvalidationListener would be added that would request the layout. For some reason, I thought this would be more efficient. Looking at it now, I think it is unnecessary and makes it less efficient. Addtionally, my understanding of the code wasn't accurate: I thought the two if statements (of whether and how to resize each scroll bar if it was visible) wouldn't work with the new set up. I thought it would need something like a switch statement: if both showing, do this; if one horizontal/vertical, do this; if none, do this. But of course, turns out I was wrong :stuck_out_tongue_closed_eyes:

[JordanMartinez]

I've removed some of the comments I made after first submitting this PR as they're no longer relevant.

[JordanMartinez]

@TomasMikula Sorry, that should have been in there. I've updated the PR to include the aforementioned comment.

[TomasMikula]

Looks good 👍

[TomasMikula]

New snapshot published.

[JordanMartinez]

Thanks!