fix: minimum start difference for same row computation (jquense#886) (jquense#909) (jquense#910)

* fix minimum start difference for same row computation (jquense#886) (jquense#909)

* Add tests for DayEventLayout

* Fix eslint on DayEventLayout

Author: bgelineau

src/DayColumn.js

@@ -165,6 +165,8 @@ class DayColumn extends React.Component {
       rtl: isRtl,
       selected,
       startAccessor,
+      step,
+      timeslots,
       titleAccessor,
       tooltipAccessor,
     } = this.props
@@ -174,6 +176,7 @@ class DayColumn extends React.Component {
       startAccessor,
       endAccessor,
       slotMetrics: this.slotMetrics,
+      minimumStartDifference: Math.ceil((step * timeslots) / 2),
     })
 
     return styledEvents.map(({ event, style }, idx) => {

src/utils/DayEventLayout.js

@@ -90,12 +90,12 @@ class Event {
 /**
  * Return true if event a and b is considered to be on the same row.
  */
-function onSameRow(a, b) {
+function onSameRow(a, b, minimumStartDifference) {
   return (
     // Occupies the same start slot.
-    Math.abs(b.start - a.start) <= 30 ||
+    Math.abs(b.start - a.start) < minimumStartDifference ||
     // A's start slot overlaps with b's end slot.
-    (a.start > b.start && a.start < b.end)
+    (b.start > a.start && b.start < a.end)
   )
 }
 
@@ -129,7 +129,7 @@ function sortByRender(events) {
   return sorted
 }
 
-function getStyledEvents({ events, ...props }) {
+function getStyledEvents({ events, minimumStartDifference, ...props }) {
   // Create proxy events and order them so that we don't have
   // to fiddle with z-indexes.
   const proxies = events.map(event => new Event(event, props))
@@ -144,7 +144,9 @@ function getStyledEvents({ events, ...props }) {
 
     // Check if this event can go into a container event.
     const container = containerEvents.find(
-      c => c.end > event.start || Math.abs(event.start - c.start) < 30
+      c =>
+        c.end > event.start ||
+        Math.abs(event.start - c.start) < minimumStartDifference
     )
 
     // Couldn't find a container — that means this event is a container.
@@ -161,7 +163,7 @@ function getStyledEvents({ events, ...props }) {
     // Start looking from behind.
     let row = null
     for (let j = container.rows.length - 1; !row && j >= 0; j--) {
-      if (onSameRow(container.rows[j], event)) {
+      if (onSameRow(container.rows[j], event, minimumStartDifference)) {
         row = container.rows[j]
       }
     }

test/utils/DayEventLayout.test.js

@@ -0,0 +1,95 @@
+import { getStyledEvents } from '../../src/utils/DayEventLayout'
+import { getSlotMetrics } from '../../src/utils/TimeSlots'
+import dates from '../../src/utils/dates'
+
+describe('getStyledEvents', () => {
+  const d = (...args) => new Date(2015, 3, 1, ...args)
+  const min = dates.startOf(d(), 'day')
+  const max = dates.endOf(d(), 'day')
+  const slotMetrics = getSlotMetrics({ min, max, step: 30, timeslots: 4 })
+
+  describe('matrix', () => {
+    function compare(title, events, expectedResults) {
+      it(title, () => {
+        const styledEvents = getStyledEvents({
+          events,
+          startAccessor: 'start',
+          endAccessor: 'end',
+          slotMetrics,
+          minimumStartDifference: 10,
+        })
+        const results = styledEvents.map(result => ({
+          width: Math.floor(result.style.width),
+          xOffset: Math.floor(result.style.xOffset),
+        }))
+        expect(results).toEqual(expectedResults)
+      })
+    }
+
+    const toCheck = [
+      [
+        'single event',
+        [{ start: d(11), end: d(12) }],
+        [{ width: 100, xOffset: 0 }],
+      ],
+      [
+        'two consecutive events',
+        [
+          { start: d(11), end: d(11, 10) },
+          { start: d(11, 10), end: d(11, 20) },
+        ],
+        [{ width: 100, xOffset: 0 }, { width: 100, xOffset: 0 }],
+      ],
+      [
+        'two consecutive events too close together',
+        [{ start: d(11), end: d(11, 5) }, { start: d(11, 5), end: d(11, 10) }],
+        [{ width: 85, xOffset: 0 }, { width: 50, xOffset: 50 }],
+      ],
+      [
+        'two overlapping events',
+        [{ start: d(11), end: d(12) }, { start: d(11), end: d(12) }],
+        [{ width: 85, xOffset: 0 }, { width: 50, xOffset: 50 }],
+      ],
+      [
+        'three overlapping events',
+        [
+          { start: d(11), end: d(12) },
+          { start: d(11), end: d(12) },
+          { start: d(11), end: d(12) },
+        ],
+        [
+          { width: 56, xOffset: 0 },
+          { width: 56, xOffset: 33 },
+          { width: 33, xOffset: 66 },
+        ],
+      ],
+      [
+        'one big event overlapping with two consecutive events',
+        [
+          { start: d(11), end: d(12) },
+          { start: d(11), end: d(11, 30) },
+          { start: d(11, 30), end: d(12) },
+        ],
+        [
+          { width: 85, xOffset: 0 },
+          { width: 50, xOffset: 50 },
+          { width: 50, xOffset: 50 },
+        ],
+      ],
+      [
+        'one big event overlapping with two consecutive events starting too close together',
+        [
+          { start: d(11), end: d(12) },
+          { start: d(11), end: d(11, 5) },
+          { start: d(11, 5), end: d(11, 10) },
+        ],
+        [
+          { width: 56, xOffset: 0 },
+          { width: 56, xOffset: 33 },
+          { width: 33, xOffset: 66 },
+        ],
+      ],
+    ]
+    toCheck.forEach(args => compare(...args))
+  })
+})