102 add scroll config api

[Gleethos]

I implemented a first draft of the scrollable configration API... You have already worked with the Scrollable interface in the framework @Mazi-S, can you take a look at it and let me know if it behaves as expected?

[Gleethos]

Note: this still needs unit tests

[Mazi-S]

I really like the approach of having a scroll config and not having to implement the scrollable interface now.

But it still does not work properly. It seems that the AddConstraint (GROW_Y) is now being ignored. I would expect the content to fill the entire scrollpane (horizontally) if present.

Or am I missing something?

UI.scrollPane(config -> config.fitWidth(true))
  .add(UI.GROW_Y, UI.panel().withLayout("")
    .withBackground(Color.LIGHT_GRAY)
    .add(UI.html(content))
  )

(I have pushed an example. Feel free to modify or remove it. 😅)

[Mazi-S]

I've introduced a new FitSizeSupplier that determines, based on the viewport and the component, if the component width/height should be forced to fit the viewport width/height.

I think this will solve a lot of problems and allow the scrollable to be used as intended. (see JTextComponent#getScrollableTracksViewportHeight)

[Gleethos]

A damn! I did not know you were also working on this at the same time! I force pushed my changes to fix the issue we were discussing in rocket... I think this overrode your commits.

Sorry for that.

[Gleethos]

I hope your local state is not yet overridden, maybe you can rebase your changes again locally and then push again.

[Mazi-S]

No problem. I still have the changes locally. 😉

In general, I like the behavior of the new MoreConsistentViewportLayout. But it is different from the Swing behavior. What I've noticed is that Swing uses getScrollableTracksViewportHeight to fit the Scrollable in the ScrollPane based on the available size (viewport) and the component size (scrollable).

In JTextComponent:

public boolean getScrollableTracksViewportHeight() {
    Container parent = SwingUtilities.getUnwrappedParent(this);
    if (parent instanceof JViewport) {
        return parent.getHeight() > getPreferredSize().height;
    }
    return false;
}

So I would suggest using a supplier that determines whether the viewport should be fitted or not instead of a simple boolean value. Similar to the ScrollIncrementSupplier.

scrollPane(it -> it
    .fitWidth(true)
    .fitHeight((viewport, component) -> viewport.getHeight() > component.getPreferredSize().height)
)

[Gleethos]

So I would suggest using a supplier that determines whether the viewport should be fitted or not instead of a simple boolean value. Similar to the ScrollIncrementSupplier.

I am not sure if I understand how this integrates with the ´Scrollable´ interface. Hmmm.. Okay so you still want to use the ´Scrollable` boolean to fit or not, but calculate this value based on a lambda provided by the user, correct?

So basically you just want to make an additional scope in the form of a lambda with a bit more context info, correct?

I that is the case I am all for it.

Please go ahead and push your chanes. I promise I will not touch the branch for now. :D

[Mazi-S]

Pushed my changes. I think this is the way Swing wants us to go. 😅

With this approach, we would not need to have a custom viewport, and the class that implements the Scrollable interface is responsible for fitting the viewport.

[Gleethos]

I really like where you are going with this, and I can already see a complete and elegant solution for this. Currently, the state also breaks some unit tests... But all it really takes now is better defaults for the scroll conf object and then we can indeed get rid of the hack I made...

I will push a and full of changes and I think then we are close to something that can be merged!

[Gleethos]

I am happy with the current implementation. Feel free to check it out and let me know if it works for you @Mazi-S

[Mazi-S]

I really like it and I think it will help a lot. I also like the default scrollable config.

One thing I noticed is a strange behavior when working directly with scrollables (e.g. using BoilerplateScrollablePanel). When you add the component with an AddConstraint, it ignores that scrollable is implemented.

.add(
    of(new BoilerplateScrollablePanel()).withLayout("wrap", "", "[]push[]")
    .withBackground(Color.LIGHT_GRAY)
    .add(html(TEXT))
    .add(html("END"))
)

.add("", // <------ only difference
    of(new BoilerplateScrollablePanel()).withLayout("wrap", "", "[]push[]")
    .withBackground(Color.LIGHT_GRAY)
    .add(html(TEXT))
    .add(html("END"))
)

I looked into the code and its because of the following line. When an AddConstraint is present, we wrap the component in a new panel, which results in the component in the scrollpane never implementing scrollable.

https://github.com/globaltcad/swing-tree/blob/fec68cb7ca6d75e6711405de99f454dc497c838e/src/main/java/swingtree/UIForAnyScrollPane.java#L18

For me, this is not a big issue because I will use the new scroll config instead of implementing the scrollable directly. But I think it is a bug and we should consider fixing it.

[Gleethos]

One thing I noticed is a strange behavior when working directly with scrollables (e.g. using BoilerplateScrollablePanel). When you add the component with an AddConstraint, it ignores that scrollable is implemented.

Ah yeah! I guess that is a bug. Although fixing this beyond just tackling the empty string constraint is a bit of a hassle, I would need to create a delegate which also implements Scrollable... I guess I can do that.

I was thinking some more about this whole fit size thing and I came up an API which is a bit easier from the users point of view in the sense that it exposes less API surface to the user while still allowing for the same functionality.

So to instead of this:

 UI.scrollPane( it -> it
      .fitWidth((viewport, view) -> viewport.getWidth()  >  view.getPreferredSize().width)
      .fitHeight((viewport, view) -> viewport.getHeight()  >  view.getPreferredSize().height)
  )
  .add(UI.GROW_Y, UI.panel().withLayout("")
    .withBackground(Color.LIGHT_GRAY)
    .add(UI.html(content))
  )

We simply use the config as a supplier for the necessary context info:

 UI.scrollPane( it -> it
      .fitWidth(it.viewport().getWidth()  >  it.view().getPreferredSize().width)
      .fitHeight(it.viewport().getHeight()  >  it.view().getPreferredSize().height)
  )
  .add(UI.GROW_Y, UI.panel().withLayout("")
    .withBackground(Color.LIGHT_GRAY)
    .add(UI.html(content))
  )

I like interfaces, but I like simple value objects with plain data more. And since we are already using a lambda to configure the scroll conf every time a Scrollable method is called we don't need an additional FitViewportSpplier.

[Gleethos]

Cool. I will merge the feature then! Feel free to use the latest version in the Framework. The SwingTree main branch should be super stable and ready to use.