stormseed / daykeep-calendar-quasar

A full event display calendar for the Quasar framework that has multiple viewing formats.
MIT License
270 stars 71 forks source link

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

Open kdmon opened 5 years ago

kdmon commented 5 years ago

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 commented 5 years ago

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

kdmon commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

sirbeagle commented 5 years ago

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 commented 5 years ago

Awesome. Thank you very much for looking into this!

sirbeagle commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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.