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

Multiple events completely hidden by another event in Day/Week views #1086

Closed
drewtonynguyen opened this issue Nov 4, 2018 · 5 comments
Closed
Labels

Comments

@drewtonynguyen
Copy link
Contributor

Do you want to request a feature or report a bug?

Bug.

What's the current behavior?

Using Big Calendar v0.20.1. step is 30, timeslots is 2.
Events are:

Event A: 1:00-3:00pm
Event B: 2:30-3:00pm
Event C: 3:00-4:30pm
Event D: 3:00-4:30pm
Event E: 4:00-5:30pm
Event F: 4:00-5:30pm
Event G: 4:30-6:00pm
Event H: 5:00-7:00pm
Event I: 5:30-7:00pm
Event J: 5:30-7:00pm
Event K: 5:30-7:00pm

I've been able to reproduce a rare case in which multiple overlapping events are hidden by a larger event.

screen shot 2018-11-04 at 10 03 00 am

As you can see, Events I and J are completely hidden by Event H.

I suspect it has something to do with the onSameRow function in DayEventLayout.js:

/**
 * Return true if event a and b is considered to be on the same row.
 */
function onSameRow(a, b, minimumStartDifference) {
  return (
    // Occupies the same start slot.
    Math.abs(b.start - a.start) < minimumStartDifference ||
    // A's start slot overlaps with b's end slot.
    (b.start > a.start && b.start < a.end)
  )
}

I see that in #910, there was a reversal in the direction of the comparison between a and b's start and end slots. However, if I add the original check back in and check both directions:

function onSameRow(a, b, minimumStartDifference) {
  return (
    // Occupies the same start slot.
    Math.abs(b.start - a.start) < minimumStartDifference ||
    // A's start slot overlaps with b's end slot.
    (b.start > a.start && b.start < a.end) ||
    (a.start > b.start && a.start < b.end)
  )
}

I end up with the following layout of events:

screen shot 2018-11-04 at 10 02 37 am

As you can see, all events are now visible. Any thoughts, @bgelineau? Would checking a and b's time slots in both directions for the same row computation be the right fix here? I can make a PR to add the check back in if so.

What's the expected behavior?

All events should be shown.

@bgelineau
Copy link
Contributor

Hi @drewtonynguyen, with your example, you are correct that a and b's times should be compared in both directions. However it has some unfortunate side-effects, like the perfectly correct :

one sided

becoming

both sides

and

2-1

becoming

2-2

From what I see, the problem stems from doing the computation of rows in the order given by sortByRender and not just in the order ('startMs', '-endMS').

@jquense : why the complex logic in https://github.com/intljusticemission/react-big-calendar/blob/master/src/utils/DayEventLayout.js#L105 given that the rows computation only require the first line? Is it to alternate the rendering between non-overlapping events of the same row like in the following screenshot in order to increase readability?

alternate

I don't see another use case and it creates the problems told above, where we have to choose between two incorrect renderings (@drewtonynguyen's being less incorrect).

@quyphan
Copy link

quyphan commented Nov 5, 2018

I am also running into the hiding of events issue and this proposed fix seems to work and make sense.

@drewtonynguyen
Copy link
Contributor Author

Thanks @bgelineau. I tested changing the sortByRender method as so, removing the logic you mentioned:

function sortByRender(events) {
  const sortedByTime = sortBy(events, ['startMs', e => -e.endMs])

  return sortedByTime
}

and my example now looks like:

screen shot 2018-11-05 at 9 58 42 am

@stale
Copy link

stale bot commented May 23, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label May 23, 2019
@stale stale bot closed this as completed May 30, 2019
@Dragomir-Ivanov
Copy link
Contributor

@drewtonynguyen It looks much nicer with your solution. @bgelineau Is there any chance, we can make this an option, so people can actually choose their sorting behavior? I will allow uglier representation, over some hidden events.

Thanks, react-big-calendar is awesome!

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

No branches or pull requests

4 participants