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

Some parallel events jump to next day in multi-day view #33

Open
kdmon opened this issue Aug 14, 2018 · 17 comments
Open

Some parallel events jump to next day in multi-day view #33

kdmon opened this issue Aug 14, 2018 · 17 comments
Assignees
Labels
bug Something isn't working

Comments

@kdmon
Copy link

kdmon commented Aug 14, 2018

There seems to be one or more formatting bugs which sets the left: css property incorrectly on parallel events. This results in some parallel events showing up on the wrong day lane in the multi-day calendar.

Looking in the source code, this issue seems related to how the events prop is parsed and in particular how these variables that are used for the css positioning are determined: thisEvent.numberOfOverlaps, thisEvent.overlapIteration, and thisEvent.overlapSegment.

quasar-cal-bug

I've gotten as far as having three and four side-by-side events showing properly by hacking the calculateDayEventStyle method like this:

if (thisEvent.numberOfOverlaps == 2 && thisEvent.overlapIteration != 1 && thisEvent.overlapSegment == 1) {
  style['left'] = (thisShift - 66.66) + '%'
} else if (thisEvent.numberOfOverlaps == 2 && thisEvent.overlapIteration == 1 && thisEvent.overlapSegment == 1) {
  style['left'] = (thisShift + 33.33) + '%'
} else if (thisEvent.numberOfOverlaps == 3) {
  style['left'] = (thisShift - 25) + '%'
} else {
  style['left'] = thisShift + '%'
}

I was hoping to see a pattern and derive a generalised fix, but so far I haven't found it. Any suggestions or explanation of these variables would be very helpful in debugging further. Thanks

@sirbeagle
Copy link
Collaborator

Yikes. Okay, I'll have to take a look at the code - I honestly can't remember how I set it up.

@sirbeagle sirbeagle self-assigned this Aug 14, 2018
@sirbeagle sirbeagle added the bug Something isn't working label Aug 14, 2018
@kdmon
Copy link
Author

kdmon commented Sep 12, 2018

FYI: I've seen some improvements by modifying the calendareventmixin.js function parseDateEvents(). This fix is much better than the previous one I posted (and which I later reverted), but this still shows some bugs on complex schedules:

parseDateEvents: function (eventArray) {
      // SORTED EVENT ARRAY
      eventArray.sort(function(a, b){return a - b})
      let overlapSegment = 1
      let overlapIteration = 1
      let n = 0
      for (let eventId of eventArray) {
        let numberOfOverlaps = 0
        for (let compareEventId of eventArray) {
          let thisEvent = this.parsed.byId[eventId]
          let compareEvent = this.parsed.byId[compareEventId]
          if (
            eventId !== compareEventId &&
            this.parseHasOverlap(thisEvent, compareEvent)
          ) {
            numberOfOverlaps++
          }
        }
        this.parsed.byId[eventId]['numberOfOverlaps'] = numberOfOverlaps
        if (numberOfOverlaps > 0) {
          // We should increment segment and reset overlapIteration
          // if the next event doesn't overlap
          if (n < eventArray.length - 1) {
            if (!this.parseHasOverlap(this.parsed.byId[eventId], this.parsed.byId[eventArray[n+1]])) {
              overlapSegment++
              overlapIteration = 1
            }
            else {
              overlapIteration++
            }
          }
          else {
            overlapIteration++
          }
          this.parsed.byId[eventId]['overlapSegment'] = overlapSegment
          this.parsed.byId[eventId]['overlapIteration'] = overlapIteration
        }
        else {
          this.parsed.byId[eventId]['overlapSegment'] = 0
          overlapSegment++
          overlapIteration = 1
        }
        n++
      }
    },

@MrDrummer
Copy link

A simple way to reproduce this is to have 3 50min long events.
1 starts at 13:00
1 starts at 13:30
1 starts at 14:00

What I would expect is two columns. 13:00 and 14:00 in one and 13:30 in the other. (another oversight here is that an event starting at the same time as one finishes causes it to go into another column)

@Jasqui
Copy link

Jasqui commented Dec 10, 2018

I know that this is an old thread and that maybe the 'quasar-calendar' isn't updated anymore but I have a solution that I used in my project that has been working pretty good so far. I wanted to share it with you guys in case anyone finds this post and is struggling with this aswell. It may not be the best possible solution or have the best performance but it works.

I ended up modifying CalendarEventMixin.js's method called "parseDateEvents" like this:

    parseDateEvents: function (eventArray) {
        let overlapIteration = 1 
        let overlapArray = []; //We are going to parse the events first as how they overlap between each other

        for (let eventId of eventArray) {
            let thisEvent = this.parsed.byId[eventId];
            let thisEventInOverlapArray = false;
            let indexOfOverlapped = -1; //I didn't use this at the end but forgot to take it out
        
            let thisEventStart = new Date(thisEvent.start.dateTime);
            let thisEventEnd = new Date(thisEvent.end.dateTime);
        
            //We iterate the overlapArray to check if the current event is in any array of those
            for (let ovIndex in overlapArray) {
                thisEventInOverlapArray = overlapArray[ovIndex].overlapped.find(ov => ov.id === eventId)
                
                if (thisEventInOverlapArray) { //If we did find it, we break out of the loop
                    indexOfOverlapped = ovIndex;
                    break;
                }

                let overlapMinStart = overlapArray[ovIndex].start;
                let overlapMaxEnd = overlapArray[ovIndex].end;


                // We check if the event date range start or end is between the range defined in the overlap object.
                // We also check if it happens to be an event that is longer than that date range and contains it.
                // If any of this is true, we proceed to add it in the overlapArray

                if ((date.isBetweenDates(thisEventStart, overlapMinStart, overlapMaxEnd)) ||
                    (date.isBetweenDates(thisEventEnd, overlapMinStart, overlapMaxEnd)) ||
                    (thisEventStart < overlapMinStart && thisEventEnd > overlapMaxEnd)) {
                    
                    overlapArray[ovIndex].overlapped.push({
                        id: thisEvent.id,
                        start: thisEvent.start.dateTime,
                        end: thisEvent.end.dateTime
                    });

                    let startDates = overlapArray[ovIndex].overlapped.map(ov => new Date(ov.start)); 
                    let endDates = overlapArray[ovIndex].overlapped.map(ov => new Date(ov.end))

                    // Now we update the range of the overlap object by getting the minimum start date and the max end date.
                    overlapArray[ovIndex].start = new Date(date.getMinDate(...startDates));
                    overlapArray[ovIndex].end = new Date (date.getMaxDate(...endDates));

                    indexOfOverlapped = ovIndex; //unused as mentioned before

                    thisEventInOverlapArray = true;
                    break;
                }
            }

            if (!thisEventInOverlapArray) { //If we didnt find it or it didnt meet the requirements to be added to an overlap object, we create a new object
                overlapArray.push({
                start: new Date(thisEvent.start.dateTime),
                end: new Date(thisEvent.end.dateTime),
                overlapped: [{
                        id: thisEvent.id,
                        start: thisEvent.start.dateTime,
                        end: thisEvent.end.dateTime
                    }]
                })
            }
        }

        //Now we go through all the overlaps and set their numberOfOverlaps and overlap]Iterations
        overlapArray.forEach(ov => {
            ov.overlapped.forEach((overlappedEvent, index) => {
                let thisEvent = this.parsed.byId[overlappedEvent.id];
                thisEvent.numberOfOverlaps = ov.overlapped.length - 1;
                thisEvent.overlapIteration = index + 1;
            })
        })
    }

EDIT: changed one of the validations from:

date.getDateDiff(thisEventStart, overlapMinStart, 'hours') < 0 && date.getDateDiff(thisEventEnd, overlapMaxEnd, 'hours') > 0)

to:

thisEventStart < overlapMinStart && thisEventEnd > overlapMaxEnd

since I found some bugs with the first one and the second one is just simpler and better imo

@kdmon
Copy link
Author

kdmon commented Dec 11, 2018

Thanks for sharing your work Jasqui. Your approach looks more robust than my quick fix. I will try out your solution when I get a chance.

@sirbeagle
Copy link
Collaborator

Trying out the code from @Jasqui - wow, nice job! I'm going to put in the test case from @MrDrummer into the demo code and use the solution above in the next release.

@MrDrummer
Copy link

Thanks for the update @sirbeagle but there's still an issue - I expected the first and last events to be in the left column since the last event starts after the first finishes.
image

I am trying to run the demo myself and test with more events to see if it will only wrap to the next column if the bottom most event's end time prevents it from snapping to the left. (as in, # 2 in your example prevents # 3 from being under # 1 due to its end time being after # 3's start.)

@MrDrummer
Copy link

Another issue is that events starting at the same time render as one wide event instead of in individual columns. I've made my own little test to demo the issues I am describing: https://github.com/MrDrummer/quasar-calendar-test

@MrDrummer
Copy link

I just fixed my issue where events at the exact same date act as one. The thisEventStart < overlapMinStart && thisEventEnd > overlapMaxEnd snippet that @Jasqui posted needs to be <= >=, so it still splits the events up if there are identical events.

@sirbeagle
Copy link
Collaborator

Okay, will take a look and try to get a fixed version out tomorrow!

@sirbeagle sirbeagle reopened this Feb 11, 2019
@sirbeagle
Copy link
Collaborator

Okay, I have a solution working on my end here but it definitely needs some more testing before I can commit it. It works very differently from the two previous versions but so far it does seems to work exactly as we're hoping, including being smart enough to fill up columns if space is available. Stay tuned.

@MrDrummer
Copy link

Awesome. Thank you very much for looking into this!

@sirbeagle
Copy link
Collaborator

Okay, v0.3.3 has been pushed which uses a very different overlap algorithm. I'm hoping it fixes everything above, but I'd love some feedback to see if it works for everyone!

@MrDrummer
Copy link

This looks like it is ideal for our needs, thanks! I'll work on integrating the changes today and will let you know how it works with lots of events :)

@matt-spx
Copy link

Thanks for the fix @sirbeagle!
While this is an improvement, I've noticed that the other events in a day with overlap will also be narrowed (regardless of whether or not there is another event at the same time).
See below:

screen shot 2019-02-19 at 14 25 07

@sirbeagle
Copy link
Collaborator

@msinkgraven Oh boogers, that's the one obvious case I didn't think of. I'll take a look and see what it'll take to correct that.

sirbeagle pushed a commit that referenced this issue Feb 20, 2019
Change overlap counting to a grid block system to better check to see how many events actually overlap. This will allow multiple column configurations within a single day column that will fully utilize available horizontal space. Affects #33. There is likely some room for optimization with the new code as it does loop and pass data through functions quite a bit.
@sirbeagle
Copy link
Collaborator

Version v0.3.4 has a slightly different overlap algorithm that, with any luck, should address @msinkgraven test case and an additional test case I caught causing problems. Mostly the same with a couple more checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants