dlsc-software-consulting-gmbh / CalendarFX

A Java framework for creating sophisticated calendar views (JavaFX 8, 9, 10, and 11)
http://www.dlsc.com
Apache License 2.0
791 stars 171 forks source link

Compute pref size height layout strategy is still using start-end time for computing overlapping #88

Closed adrianjaroszewicz closed 4 years ago

adrianjaroszewicz commented 4 years ago

I would like to calculate the height of an entry myself, basing on the amount of text that has been added to the titleLabel. In order to achieve that, I have added an extension of DayEntryView:

public class DetailDayEntryView extends DayEntryView
{
    public DetailDayEntryView( Entry< ? > entry )
    {
        super( entry );
        setHeightLayoutStrategy( EntryViewBase.HeightLayoutStrategy.COMPUTE_PREF_SIZE );
    }

    @Override
    protected double computePrefHeight( double width )
    {
        Label titleLabel = ((Label)lookup( ".title-label" ));
        if( titleLabel != null )
        {
            return titleLabel.prefHeight( width );
        }
        return super.computePrefHeight( width );
    }
}

that I use in my own EntryViewFactory:

public class DayEntryViewFactory implements Callback< Entry< ? >, DayEntryView >
{
    ...
    @Override
    public DayEntryView call( Entry< ? > anEntry )
    {
        return new DetailDayEntryView( anEntry );
    }

and this code works great in most scenarios, the height of an entry is the same as height of a label.

The problem appears when entries are overlapping. As far as I can see overlapping of entries is still based on their startTime and endTime - so even though an entry can fit in the view after reducing its height, it is still being rendered as it was overlapping with another one:

calendarfx_overlapping_problem

(dotted lines show the size of the entry when the end time is taken into account)

I understand that this problem is not trivial. In the example above the height of an entry is less than using endTime so there is no overlapping - but when we change the height of an entry so that it is greater than height based on startTime and endTime, some additional overlapping can appear. In that case the width of the label decreases and basing on that, its height increases - so in order to present the whole text to the user, we would have to repeat the process until each entry has a proper height. It is our problem though - and I already have an idea how to solve it (I would have to introduce the maxEntryHeightProperty).

But I cannot do it since endTime is not bound with the current height of the entry when using HeightLayoutStrategy.COMPUTE_PREF_SIZE strategy.

PS. I have already tried to use com.calendarfx.model.Entry#changeEndTime(java.time.LocalTime) but after doing that I receive lots of exceptions:

java.lang.IndexOutOfBoundsException: Index 55 out of bounds for length 55 at jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64) ~[?:?] at jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70) ~[?:?] at jdk.internal.util.Preconditions.checkIndex(Preconditions.java:248) ~[?:?] at java.util.Objects.checkIndex(Objects.java:372) ~[?:?] at java.util.ArrayList.get(ArrayList.java:458) ~[?:?] at com.sun.javafx.collections.ObservableListWrapper.get(ObservableListWrapper.java:89) ~[javafx-base-11.0.2-PSI-ALPHA-8-win.jar:?] at com.sun.javafx.collections.VetoableListDecorator.get(VetoableListDecorator.java:306) ~[javafx-base-11.0.2-PSI-ALPHA-8-win.jar:?] at javafx.scene.Parent.layout(Parent.java:1210) ~[javafx-graphics-11.0.2-PSI-ALPHA-8-win.jar:?] at javafx.scene.Parent.layout(Parent.java:1213) ~[javafx-graphics-11.0.2-PSI-ALPHA-8-win.jar:?]

with no reference to any mine or calendarfx code.

dlemmermann commented 4 years ago

I have added the property autoLayout to DayViewBase, which means it is also available for the ResourceCalendarView. There the value is set to false, so CalendarFX will no longer try to place entries with overlapping time intervals in separate columns. Please let me know if this solves this issue.

adrianjaroszewicz commented 4 years ago

Unfortunately it doesn't help much - at the moment in both scenarios, autoLayout set to true or false, we cannot see all the entries:

calendarfx_overlapping_problem2

It looks like when entries have the same start time, the following situation happens:

In both scenarios other entries are underneath.

Our goal is not to eliminate the rendering of overlapping, but base it on the height of the entry and not the start-end time information (when using HeightLayoutStrategy.COMPUTE_PREF_SIZE strategy). I guess that it should be probably done by binding the endTime of the entry with its height when this strategy has been chosen.

Btw. Is this new behavior of entries when the autoLayout is set to false intended? I would expect to see the overlapping (entries next to each other) when entries collide with each other (as it was being drawn before) and not being hidden underneath others.

adrianjaroszewicz commented 4 years ago

It looks like we've found the alternative way of solving this issue. Could you please wait with developing any new solutions? I will test it and let you know if the solution we thought of is enough for us.

The only valid question is the last one, regarding the autoLayout set to false.

dlemmermann commented 4 years ago

Actually I am right in the middle of changing the boolean “autoLayout" property to an object property storing an enum called OverlapCriteria. This enum would contain the values NONE, TIME_BOUNDS, VISUAL_BOUNDS.

On 30 Apr 2020, at 12:18, adrianjaroszewicz notifications@github.com wrote:

It looks like we've found the alternative way of solving this issue. Could you please wait with developing any new solutions? I will test it and let you know if the solution we thought of is enough for us.

The only valid question is the last one, regarding the autoLayout set to false.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dlsc-software-consulting-gmbh/CalendarFX/issues/88#issuecomment-621744338, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACIXWXKQDPB5LFY6XI3DFCLRPFF7DANCNFSM4MQDRPSA.

adrianjaroszewicz commented 4 years ago

Oh, okay, that sounds very promising - we'll wait for the results then :) thank you!

dlemmermann commented 4 years ago

I have added the method DayViewBase.setOverlapResolutionStrategy(). For those text entries please set the enum value OverlapResolutionStrategy.VISUAL_BOUNDS. Let me know if this makes you move forward.

adrianjaroszewicz commented 4 years ago

Wow, it looks amazing now! Outstanding job, that was exactly what I was looking for :) thank you!

calendarfx_overlapping_problem_fixed

I have noticed just one strange behaviour: when the start time of the entry is before the visible area, its width decreases and entries no longer fill the available width of the view. Could you take a look at it?

calendarfx_overlapping_problem3

Just wanted to let you know that since we have a public holiday tomorrow in Poland, I will be able to answer you on Monday. Thank you once again and have a nice weekend!

dlemmermann commented 4 years ago

Glad you like it. I think this is a very useful addition to CalendarFX anyways.

I will have a look at the issue you mention.

Have a good holiday.

Dirk

On 30 Apr 2020, at 17:43, adrianjaroszewicz notifications@github.com wrote:

Wow, it looks amazing now! Outstanding job, that was exactly what I was looking for :) thank you!

https://user-images.githubusercontent.com/34687474/80728001-b884d880-8b06-11ea-8082-5220046464f7.png I have noticed just one strange behaviour: when the start time of the entry is before the visible area, its width decreases and entries no longer fill the available width of the view. Could you take a look at it?

https://user-images.githubusercontent.com/34687474/80729526-ac9a1600-8b08-11ea-88bb-07201964b30e.gif Just wanted to let you know that since we have a public holiday tomorrow in Poland, I will be able to answer you on Monday. Thank you once again and have a nice weekend!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dlsc-software-consulting-gmbh/CalendarFX/issues/88#issuecomment-621935681, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACIXWXLEXG7DDOXYQO45WFTRPGMC5ANCNFSM4MQDRPSA.

dlemmermann commented 4 years ago

There seem to be two issues here. The first one is that the calendar model returns the same entry twice if an entry spans two days (crosses midnight). The other one is an error in calculating those overlapping entry clusters and the number of columns required by a cluster. Issue 1 is fixed and I pushed the changes. Issue 2 still needs to be resolved.

dlemmermann commented 4 years ago

This is fixed now. The cluster calculation was indeed wrong.

adrianjaroszewicz commented 4 years ago

Thank you, both these things work fine now :)