Need a critical eye for Carousel control

[John Hendrikx]

Hi list,

I've found some more motivation to work on a Carousel style control, and I've made a lot of progress.  However, I'm in need of feedback as to the direction I've taken with the control.  My aim has been to allow a lot flexibility -- and it turns out that these kinds of controls have options in abundance to tweak their look :/

The control is basically a Skin for the TreeView control, and not a new control at all.  Most of the selection, focus and navigation handling is perfectly suitable for a carousel, although perhaps choosing TreeView as a basis for it is a bit overly ambitious (I have some plans in that regard).

Current features:

- Works with TreeCells, with some restrictions and oddities (depending on layout)

- Smooth scrolling

- Reflections (without or with clipping to make them look even better)

- Multiple different layouts (flat ribbon, 3d images directed towards center, 3d images in a circular pattern)

- Layouts allow for customization (like fade in/out of edge cells, or different cell rotation styles)

- Layouts can also written from scratch, allowing for layouts that I haven't even thought of yet

- Almost a dozen properties that can be tweaked (alignment, maximum size of cells, reflections, radius, etc)

The Skin class has several common JavaFX properties and a Layout property.  The Layout provides more specific JavaFX properties and provides the Skin with the exact cells to position depending on the Carousel's current state -- they make use of a CellIterator instance to be able to keep track of important state when calculating the positions of individual cells and how they relate to other cells (depending on layout).  Customization is available at the Layout level either by writing your own implementation or by subclassing an existing one.  

The design feels still somewhat inflexible to me mostly because of how Reflections (and their clipping) come into play during the calculations.  Since adding a Reflection effect on a Node changes the size of the Node it is not something that could easily be added as an optional post processing step (especially when combined with the PerspectiveTransform to make the cells appear 3d).  This control could really use some way to create reflections, without using the Reflection effect, to simplify the design.

I'd very much appreciate some feedback... I think this could be turned into a full fledged control.

To test, simply get the latest version (which works on JavaFX 8-b80) from GitHub at https://github.com/hjohn/Carousel/tree/javafx8 and run the TestCoverflow class.  This also has some more documentation in the comments.

--John

[Jonathan Giles]

Hi John,

Sorry for not answering your much earlier private email regarding this
topic. I've been a little snowed under with work. Nonetheless, I'll try
to give some feedback now.

To start with I best deal with the elephant in the room, namely, using a
TreeView control / API as a model for a Carousel control doesn't seem
intuitive to me (although I admit I have spent no time thinking about
the requirements for a Carousel). To me, a ListView would perhaps have
formed a better foundation (namely, what does the hierarchical nature of
a TreeView (with its TreeItems) give you?). I'm assuming perhaps your
intention is to have a hierarchical carousel that allows for clicking
into and out of the hierarchy to browse different categories (in which
case I can appreciate there is some value in this structure). At the
very least the ability to show the root does seem odd.

Now on to the bullet point list of questions / comments (and please keep
in mind I haven't cloned the repo)

* What do the SimpleCellPool / CellPool classes provide? It seems like
perhaps you're duplicating functionality already implemented by the
TreeView.

* It seems odd to me that you are not exposing a Carousel control
class, for a number of reasons:
* For starters by using carousel.css to replace the skin of all
.tree-view instances you're essentially clobbering the ability for a
default TreeView (i.e. one that uses TreeViewSkin). I would recommend
having a Carousel class that has a default style class of 'carousel' -
and do not modify the properties of .tree-view and .tree-cell in your
CSS. If you must modify .tree-cell, make it so that it is only for
descendants of .carousel (e.g. something like .carousel .tree-cell {....})
* Also, it means you require users to depend on the TreeItem API
(which as I've noted already feels odd - what is the root of a Carousel
and what are its children?).

* It is hard for me to appreciate what you deem is public API and what
is private API (in the same way we have javafx.scene.control and
com.sun.javafx.scene.control). It would be great if you would consider
separating out your js.javafx.control package into public and private
API. Obviously the intention should be to have as little in the public
package as possible. Similarly, I would suggest splitting your demo code
into a separate package (TestCoverFlow, maybe ControlPanel?)

* It is hard to understand the value of the CellIterator and Layout
code. This is mostly my fault for not loading the code up in an IDE, but
in general it might pay to consider whether this is necessary or over
engineering (and I'm not suggesting it is, I'm merely asking whether it
is necessary).

I think that is a good starting point, so I'll leave it there.

-- Jonathan

> --
> You received this message because you are subscribed to the Google
> Groups "JFXtras Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to jfxtras-dev...@googlegroups.com.
> To post to this group, send email to jfxtr...@googlegroups.com.
> Visit this group at http://groups.google.com/group/jfxtras-dev?hl=en.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

[John Hendrikx]

On Sunday, 17 March 2013 02:54:28 UTC+1, Jonathan Giles wrote:

Hi John,

To start with I best deal with the elephant in the room, namely, using a
TreeView control / API as a model for a Carousel control doesn't seem
intuitive to me (although I admit I have spent no time thinking about
the requirements for a Carousel). To me, a ListView would perhaps have
formed a better foundation (namely, what does the hierarchical nature of
a TreeView (with its TreeItems) give you?). I'm assuming perhaps your
intention is to have a hierarchical carousel that allows for clicking
into and out of the hierarchy to browse different categories (in which
case I can appreciate there is some value in this structure). At the
very least the ability to show the root does seem odd.

You are right, this skin so far is an experiment, and the current functionality would much better match ListView.  I'm still thinking about how I could make the carousel hierarchical (for my own use case).  There's another reason however, ListView basically is a subset of TreeView's functionality and when I started it I didn't see a good reason to limit my options.  It should be easy enough to make it a ListViewSkin.

Now on to the bullet point list of questions / comments (and please keep
in mind I haven't cloned the repo)

  * What do the SimpleCellPool / CellPool classes provide? It seems like
perhaps you're duplicating functionality already implemented by the
TreeView.

They are there to manage the cells and provide re-use of the cells efficiently.  I externalized the code as I cannot foresee the needs of different carousel implementations.  It is a duplication as I donot know how to capitalize on the code for this in TreeView/VirtualFlow (I'll also admit I donot fully understand how it works).  All I really need is what you can see in the CellPool interface -- some way to get a cell by index, and having it keep track which cells are in use so that it won't re-use a cell that I asked for in the same layout pass.

Speaking of duplication, I had to duplicate most of the code in TreeUtil as well, as I cannot access it -- possibly this is also because I could not use the VirtualFlow method of doing things.

Things that might make it hard to use VirtualFlow:

- I need to be able to access the children of the TreeView directly so I can sort them in the correct order (as the center most cell must be the top most cell).

    List<Node> children = new ArrayList<>(getChildren());

    Collections.sort(children, Z_ORDER_FRACTIONAL);

    getChildren().setAll(children);

- I need to be able to provide cells in a very specific order (from closest Z to furthest away Z -- typically it is something like: cell 0, cell 1, cell -1, cell 2, cell -2, cell 3, cell -3).  This is required because of the way Reflections must be clipped to prevent them blending.  See the layoutChildren() method in CarouselSkin and look at the cumulativeClip variable.

- This TreeViewSkin has a center or focused cell approach when it comes to keeping track of position (there is no global coordinate for a cell with respects to the entire TreeView).  It only knows which cell must be positioned in the center and it will position more cells on demand to the left and to the right until enough are present to fill the view (although some Layouts have a stable number of cells no matter what).  This means that the number of cells in use can differ with each JavaFX pulse and is not just dependent on the size of the control.  It also means that the center cell is stable -- if it changes dimensions or cells nearby change dimensions, it will have no influence on the position of the center cell.

I don't know if all this can be achieved while using the cell management present in TreeView, but if it could then I'd prefer to use it.

 

  * It seems odd to me that  you are not exposing a Carousel control
class, for a number of reasons:
    * For starters by using carousel.css to replace the skin of all
.tree-view instances you're essentially clobbering the ability for a
default TreeView (i.e. one that uses TreeViewSkin). I would recommend
having a Carousel class that has a default style class of 'carousel' -
and do not modify the properties of .tree-view and .tree-cell in your
CSS. If you must modify .tree-cell, make it so that it is only for
descendants of .carousel (e.g. something like .carousel .tree-cell {....})

Well, it is just a skin, so where it gets used is up to the user.  The carousel.css is part of the demo -- and it takes the easy way out.  It is not my intention to clobber all default TreeView's.

I donot want to touch .tree-cell at all -- this is only done to work around an NPE (related to disclosure node being null) and to get rid of some unwanted padding (which is added I believe by TreeCellSkin regardless of whether there is a disclosure node or not).  That was a few beta's ago, I'll see if it is fixed now.   Of course, the obvious answer would be to switch to ListViewSkin to solve these :)

 

    * Also, it means you require users to depend on the TreeItem API
(which as I've noted already feels odd - what is the root of a Carousel
and what are its children?).

It can be changed to ListView or I can provide both.  It's best to disregard the dependency on TreeView for now, I had no special reason to choose one or the other, apart from some hierarchical carousel ideas that I may need in my own application.  The code does not care whether it is dealing with TreeCell or something else.

  * It is hard for me to appreciate what you deem is public API and what
is private API (in the same way we have javafx.scene.control and
com.sun.javafx.scene.control). It would be great if you would consider
separating out your js.javafx.control package into public and private
API. Obviously the intention should be to have as little in the public
package as possible. Similarly, I would suggest splitting your demo code
into a separate package (TestCoverFlow, maybe ControlPanel?)

Alright, I'll try and make this more clear, although I'm unsure what I would want to make private API -- I guess those pieces I'm not quite happy with and that may see change in the future.  Other than that, given the flexibility I want to provide, most of the classes are public in one way or another.  However, I can split them up in classes that someone will use when only using the Skin with minimal customization, and for someone who wants to make their own version of a carousel style control (by implementing a Layout).

  * It is hard to understand the value of the CellIterator and Layout
code. This is mostly my fault for not loading the code up in an IDE, but
in general it might pay to consider whether this is necessary or over
engineering (and I'm not suggesting it is, I'm merely asking whether it
is necessary).

It could be over engineered, but I think I had good reason -- it IS possible to provide multiple different layouts by just providing more Skins (or by subclassing an abstract Skin that provides the basic functionality).  However, the Skin API is not nearly as flexible and easy to use when you want to alter the layout of a carousel slightly.

Also, I donot think you can switch Skins as easily as you can switch Layouts.  For example, the Carousel can be viewed in multiple different layouts.  This is done with a ComboBox that is bound like this:

    carouselSkin.layoutProperty().bind(comboBox.getSelectionModel().selectedItemProperty());

The purpose of the Layout class is to be easily subclassed (even anonymously) to make a small adjustment to how that particular layout will look that cannot be easily provided in a Property.  It should have some descriptive protected methods available by default that are easy to understand for a user subclassing it (like fadeOutEdgeCells or rotateCenterCellsTowardsViewer).  The CellIterator class works in close conjunction with Layout as this provides you with the state you need to make decisions on how to change the layout.  It will allow you to see if the Cell currently being positioned is near the edge for example, giving you the option of making it fade out (or explode into pieces...) -- Iterator seemed the logical pattern for this as it can contain both global state for a single layout pass as well as state for the current cell.

Doing all of this in a Skin would create a lot of (protected?) fields that are only used during the layout -- I didn't like that very much, although the single thread model ensures me that nothing can go wrong by just initializing those fields at the start of a layout and using them only then, it just felt wrong to me to use fields like that.  Furthermore, it makes it much harder than needed to alter functionality slightly.  Subclassing the Skin directly provides way more API than needed to just make slight changes and overriding the wrong method would just break everything.      

Below an example of customization (not too hard, although you need to know what you are doing):

The default RayLayout class provides a layout where all cells are laid out like spokes of a wheel, fading in/out the first/last visible cells and rotating the cells near the center so it can be viewed front on.  Changing this to a ring of cells that are always facing outwards (instead of like spokes) only requires you to override a method in RayLayout:

      myCustomLayout = new RayLayout(carouselSkin) {

        @Override

        protected void customizeCell(RayCellIterator iterator) {

          Point3D[] points = iterator.currentPoints();  // Coordinates of the current cell rectangle in 3D

          Point3D center = new Point3D((points[0].getX() + points[1].getX()) * 0.5, 0, (points[0].getZ() + points[1].getZ()) * 0.5);  // Point to rotate around

          for(int i = 0; i < points.length; i++) {  // Loop through all coordinates

            points[i] = rotateY(points[i], center, Math.PI * 0.5);  // Rotate 90 degrees

          }

          this.fadeOutEdgeCells(iterator, 0.5);  // Call the standard fade in/out with a fade in/out distance of half a cell

        }

      }

I could have provided a SpokeBasedCarouselSkin, a RingCarouselSkin, a FrontOnCarouselSkin... there are so much options when it comes to placement, rotation, fade in/outs, scaling... I just didn't know what to do -- providing those as properties would explode the number of properties.  Allowing parts of the Skin to be overridden and subclassed easily seemed the only way I could provide a large variety of styles, without limiting them to just my own imagination.

 

I think that is a good starting point, so I'll leave it there.

Much appreciated.  I'll see if I can address some of the points in code this weekend. 

--John

[Miachael Annie]

Very interesting information, this reminded me of the UI carousel control, and C# treeview control. I am willing to take a try.

[John Hendrikx]

Hi,

You can have a try, the control is on GitHub (use the javafx8 branch, master branch is old).  Let me know if you have any problems with it.

I've updated the control in the mean time, and changed it to use multiple skins for different styles (3 styles are available, Ribbon, Ray (3d) and Flat).  It is not quite JFXtras material yet, but should be quite usable.

--John

On 24 March 2014 07:23, Miachael Annie <xiaoy...@gmail.com> wrote:

Very interesting information, this reminded me of the UI carousel control, and C# treeview control. I am willing to take a try.

--
You received this message because you are subscribed to a topic in the Google Groups "JFXtras Developers" group.

To unsubscribe from this topic, visit https://groups.google.com/d/topic/jfxtras-dev/AuBcmMf_ZHQ/unsubscribe.

To unsubscribe from this group and all its topics, send an email to jfxtras-dev...@googlegroups.com.

To post to this group, send email to jfxtr...@googlegroups.com.

Visit this group at http://groups.google.com/group/jfxtras-dev.
For more options, visit https://groups.google.com/d/optout.