React-calendar-timeline: Rework itemRenderer prop

[Ilaiwi]

Hi,

I am working on a schedule to manage employee shifts. The library till now is working really well (Thank you for the effort). But the main issues I am having is that we don't have access to render all of the items differently. So my code to manage Items and how they should look in different scenarios was split into two parts: First part is a custom Item component which handles the rendering of the inner div of the item and the class name property in the array items passed to the component which I find awkward.

I understand that you had to do that in order to guarantee that the item renders with the correct height and left values and handlers

My suggestion is based on the renderProp pattern that react-beautiful-dnd employes, where the library trusts the developer to give those key attributes to the parent div via an argument called provided. This will give developers who need to do highly custom item to do so.

please refer to react beautiful dnd droppable for an example

I think user code will be something like this:

<Timeline groups={groups}
    items={items}
    defaultTimeStart={moment().add(-12, 'hour')}
    defaultTimeEnd={moment().add(12, 'hour')}
    fullItemRenderer={(item, provided, providedDragLeft, providedDragRight) => (
        <div
            ref={provided.getInnerRef}
            // this includes the key properties like `left` and `height` and handlers (drag, click... handlers)
            {...provided.itemProps}
        >
            <div
                ref={providedDragLeft.getInnerRef}
                {...providedDragLeft.itemProps}
            >
            </div>
                {item.title}
            <div
                ref={providedDragRight.getInnerRef}
                {...providedDragRight.itemProps}
            >
            </div>
        </div>
    )}
/>

of course this prop will be optionals and will replace itemRenderer when it only excists

P.S. I have the resources to develop this feature if you think it is a good idea.

[mcMickJuice]

This is a fantastic idea.

where the library trusts the developer to give those key attributes to the parent div via an argument called provided. This will give developers who need to do highly custom item to do so.

100% yes. react-virtualized does this with some of their components where all positioning is passed into the render function. Giving more render control and responsibility to the user has become a mantra of React libraries over the past year or so as well.

And you're right, this will and should replace itemRenderer. I imagine some users like the fact that you can just provide a item with a title property and it will Just Work but I believe there are complications and limitations with that approach. This is a major breaking change too but oh well. This is OSS.

It'd be great if you could contribute this feature. My suggestions:

• check out how downshift handles passing props via its render function. It allows you to compose event handlers which is something this library sorely needs (onItemClick handlers are really wonky)
• make sure that dragging, resizing and all the item events are working. But more importantly, do not feel shy about refactoring how item interactions work
• also make sure you're passing in timelineContext as a prop

Feel free to reach out with any questions you may have.

[Ilaiwi]

@mcMickJuice Thank you! I will start with this very soon under prop name fullItemRenderer so maybe we don't push this as a breaking change for now?

[mcMickJuice]

No, I'd prefer we just replace itemRenderer and include instructions for migrating from the previous version.

[Ilaiwi]

Hi, I started with this task by creating a new branch from develop because I saw your comment on #283 . I noticed there was a bug when moving the items between groups as shown here. Not sure If I should continue from develop or switch to mater?

[mcMickJuice]

Yikes, I'll look at that bug. Continue working on your branch (off of develop) and we can merge afterward

[mcMickJuice]

@Ilaiwi fix should be on develop. please merge when you have a chance.

Thanks for pointing this out!

[Ilaiwi]

@mcMickJuice Hi, I implemented the render prop with prop getter pattern to render the items and made the default prop the previous render function but I had problems wasn't as snappy when moving items as the current version of master (0.16v). I also tried the provide patterns since it has fewer function calls but it was slower as well. Please Take a look at my fork. Please, let me know if you have any idea on how to maintain the performance or your thoughts on the performance hit.

[Ilaiwi]

@mcMickJuice I did some optimizations using why-did-you-update on master. There was a lot of extra renders on every interaction but now performance is snappier (please compare the two branches with 6X throttle) (optimizations branch). I then merged the optimizations with my itemRenderer branch and it resulted in a similar performance on the current master. There are some extra optimizations to be done using the tool but I thought maybe you should look at the optimizations first

[mcMickJuice]

Great! Can you open a PR against master so I can review?

[Ilaiwi]

@mcMickJuice to be clear with only the optimizations?

[mcMickJuice]

Can you open a PR for the itemRenderer prop reworking only - this branch? Not with the optimizations? It's good practice to keep PRs very specific.

If you can open up an issue to discuss the optimizations you made along with a separate PR, that'd be awesome too!

[mcMickJuice]

This has been published to @beta . I'm going to merge other substantial changes first then publish to 0.19.0. Also gotta add a migration from existing item renderer and this one.

Thanks @Ilaiwi !

[mcMickJuice]

Released in 0.19.0