Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pinning an entry on top of other entries #110

Closed
adrianjaroszewicz opened this issue Oct 15, 2020 · 12 comments
Closed

Pinning an entry on top of other entries #110

adrianjaroszewicz opened this issue Oct 15, 2020 · 12 comments

Comments

@adrianjaroszewicz
Copy link

Hello Dirk,
I would like to ask you about the possibility of drawing an entry on top of other entries (in ResourceCalendarView). As far as I know there is no such possibility at the moment - am I right?

Would you consider adding it? The given entry would not be taken into account while calculating the overlapping (it would always stay on the top):
calendarfx_entry_on_top

The API I imagine could look like com.calendarfx.view.EntryViewBase#setOnTop(boolean) - so that we could use it in our extension of EntryViewBase like this:

setOnTop(true);
setPrefWidth(50);
setAlignmentStrategy(AlignmentStrategy.ALIGN_RIGHT);

What do you think about it?

Cheers,
Adrian

@dlemmermann
Copy link
Collaborator

dlemmermann commented Oct 15, 2020 via email

@adrianjaroszewicz
Copy link
Author

Sure, that makes sense to me :) I will contact my colleagues and will let you know about our decision.

Have a nice day!
Adrian

@adrianjaroszewicz
Copy link
Author

Hello Dirk,

I'm sorry for not contacting you in this manner but our customer has postponed the development of this feature for some time. But after a couple of months he came back to it - so our management has decided to go with the first option that you have suggested.

We have an idea of how the implementation could look like - but we wanted to ask for your feedback first, in order not to go in a wrong direction.

The idea is to add a new method impl.com.calendarfx.view.DayViewSkin#layoutEntryViewsOnTop, here is a draft implementation:

private void layoutEntryViewsOnTop(List<DayEntryView> entryViews, DayView dayView, double contentX, double contentY, double contentWidth, double contentHeight) {
	List<DayEntryView> entryViews = entryViews.stream().filter(EntryViewBase::isOnTop).collect(Collectors.toList());
	for(DayEntryView entryView : entryViews)
	{
		Entry<?> entry = entryView.getEntry();

		double y1;
		double y2;

		if (dayView.isScrollingEnabled()) {

			y1 = dayView.getLocation(entry.getStartAsZonedDateTime());
			y2 = dayView.getLocation(entry.getEndAsZonedDateTime());

		} else {

			y1 = dayView.getLocation(entry.getStartTime());
			y2 = dayView.getLocation(entry.getEndTime());
		}
		double minHeight = entryView.minHeight(contentWidth);
		entryView.resizeRelocate(snapPositionX(contentX + 60), snapPositionY(y1),
				snapSizeX(contentWidth / 2), snapSizeY(Math.max(minHeight, y2 - y1 - 2)));
		entryView.toFront();
	}
}

and call it after each call of impl.com.calendarfx.view.DayViewSkin#layoutEntryViews. Additionaly, the list of entry views that are given as an input to the impl.com.calendarfx.view.DayViewSkin#layoutEntryViews method would be filtered so that entries that are on the top would not be passed there.

Of course this is just a draft, in the final solution we would take the alignment information into account (com.calendarfx.view.EntryViewBase#getAlignmentStrategy) + a property that tells what should be the width of this entry (we were thinking about introducing a property that keeps the information about a percentage width of this entry in the context of the whole column). It would be added to the EntryViewBase class:

    private final DoubleProperty widthPercentage = new SimpleDoubleProperty(this, "widthPercentage", 100) {

        @Override
        public void set(double newValue) {
            if (newValue < 10 || newValue > 100) {
                throw new IllegalArgumentException("percentage width must be between 10 and 100 but was " + newValue);
            }
            super.set(newValue);
        }
    };

    /**
     * A percentage value used to specify how much of the available width inside the
     * view will be utilized by the entry views. The default value is 100%, however
     * applications might want to set a smaller value to allow the user to click and
     * create new entries in already used time intervals.
     *
     * @return the entry percentage width
     */
    public final DoubleProperty widthPercentageProperty() {
        return widthPercentage;
    }

    /**
     * Sets the value of {@link #widthPercentage}.
     *
     * @param percentage the new percentage width
     */
    public final void setWidthPercentage(double percentage) {
        this.widthPercentage.set(percentage);
    }

    /**
     * Returns the value of {@link #widthPercentageProperty()}.
     *
     * @return the percentage width
     */
    public double getWidthPercentage() {
        return WidthPercentage.get();
    }

What do you think about this direction? Is this something you would like to include to calendarx? We are opened to any suggestions so please let me know what's your opinion :)

Cheers,
Adrian Jaroszewicz

@dlemmermann
Copy link
Collaborator

dlemmermann commented Mar 22, 2021 via email

@adrianjaroszewicz
Copy link
Author

Thank you for the feedback! Do you mean impl.com.calendarfx.view.util.VisualBoundsResolver#resolve and impl.com.calendarfx.view.util.TimeBoundsResolver#resolve? Yes, that is probably a good idea to filter out entries that are not on top inside both these methods (and not only change the input to the impl.com.calendarfx.view.DayViewSkin#layoutEntryViews method) - in order anyone wants to use these resolve methods outside of the DayViewSkin.
Ok, so we are starting the development - and will let you know about the results.
Have a nice day!
Adrian

@dlemmermann
Copy link
Collaborator

Yes, those are the ones I meant.

@adrianjaroszewicz
Copy link
Author

Hello Dirk,

we have prepared a pull request with changes that introduce the possibility of putting entries on top of other entries. Could you please look at it and check if this is acceptable for you and can be merged to CalendarFX?
#123

Here is our reasoning behind the changes:

  • instead of introducing "onTop" property, we have decided to create "layer" property, we hope the code will be more readable this way
  • we split entries for layers before calling layout methods, each layer (BASE and TOP) now has its own layout method. Top layer uses simpler layout mechanics of simply placing items, without resolving overlapping entries.
  • new widthPercentage property is only used for view entry width computation if no preferred width value is present
  • in Swimlane mode, we are grouping entries for each calendar before layouting them, instead of filtering them for each calendar in the loop (it seems more efficient this way)
  • the data in ResourceCalendarApp is now generated from seed, each time the example app is started the data should look the same if seed has not been changed

Also, while implementing the new feature, we have found two things that possbily might be bugs.

  1. When using Alignment Strategy different than FILL and not defining value of preferred width of view entry, all entries have no width, and are not visible.
  2. When using Alignment Strategy ALIGN_RIGHT in Swimlane mode, the offset of entry was not correctly computed:
	switch (entryView.getAlignmentStrategy()) {
		case ALIGN_RIGHT:
			x = columnWidth - w;
			break;

the correct formula probably is: x = x + (columnWidth - w).
Our changes are addressing those two problems:
In 1) when no preferred width value is present, we simply use all available space (columnWidth or contentWidth).
In 2) the created function adds column offset.

Regarding the filtering in resolver classes, we have introduced it in layoutStandard and layoutSwimlane methods. My friends suggested that I might have misunderstood you and it should be enough to have the filtering there and not introduce it additionally in resolver classes - but please let me know if our understanding is right here.

Of course please let me know in case of any comments/suggestions. I hope the code is fine and meets the standards of CalendarFX project :)

Cheers,
Adrian

@dlemmermann
Copy link
Collaborator

PR looks pretty good already. Please see my comments there. I think it is important to have at least one sample app that highlights this feature and also do not use it in the samples that are already there.

@adrianjaroszewicz
Copy link
Author

Sure thing, we added the sample. I hope it's fine - if you could take a look that would be great.

@adrianjaroszewicz
Copy link
Author

Sorry for updating but it looks like our customer really needs this feature - so we would be grateful if you could check it before Easter. Do you think it is possible? If not and you're already on vacation, please don't bother, we can wait :)

@dlemmermann
Copy link
Collaborator

I won't be able to get to it. But whatever the final PR, it will support layers. Until then why don't you just stay on your fork?

@adrianjaroszewicz
Copy link
Author

That makes sense, we're going to go this direction :) Happy Easter!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants