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

RFC: row renderer #656

Open
Ilaiwi opened this issue Sep 24, 2019 · 27 comments
Open

RFC: row renderer #656

Ilaiwi opened this issue Sep 24, 2019 · 27 comments
Labels
feature-request rowRenderer might solve the issue

Comments

@Ilaiwi
Copy link
Collaborator

Ilaiwi commented Sep 24, 2019

Is your feature request related to a problem? Please describe.

Add an API to the calendar part of the timeline. This API would give you control to add custom UI on calendar rows using a render prop. You can control what is rendered by default with the library like Items and Vertical/Horizontal lines, and the renderer will provide you the ability to render custom backgrounds and droppable layers for custom dnd

Describe the solution you'd like

<Timeline
    groups={groups}
    items={items}
    rowRenderer={({ rowData, helpers, getLayerRootProps, group }) => {
        const { itemsToDrag, unavailableSlots, timelineLinks } = rowData
        const groupUnavailableSlots = unavailableSlots[group.id]
        ? unavailableSlots[group.id]
        : []
        return (
        <>
            <UnavailableLayer
                getLayerRootProps={getLayerRootProps}
                getLeftOffsetFromDate={helpers.getLeftOffsetFromDate}
                groupUnavailableSlots={groupUnavailableSlots}
            />
            <DroppablesLayer
                getLayerRootProps={getLayerRootProps}
                itemsToDrag={itemsToDrag}
                getLeftOffsetFromDate={helpers.getLeftOffsetFromDate}
                handleDrop={this.handleDrop}
                group={group}
            />
        </>
        )
    }}
    rowData={{
        itemsToDrag: this.state.itemsToDrag,
        unavailableSlots: this.state.unavailableSlots,
        timelineLinks: this.state.timelineLinks,
    }}
/>

Describe alternatives you've considered

keep the plugin system (undocumented)

Additional context

Check out the full RFC:
https://github.com/namespace-ee/react-calendar-timeline/blob/rowRenderer/rfcs/row-renderer/index.md

Checkout prototype at the row-renderer branch on the repo

@Ilaiwi Ilaiwi added feature-request rowRenderer might solve the issue labels Sep 24, 2019
@Ilaiwi
Copy link
Collaborator Author

Ilaiwi commented Sep 24, 2019

I need so opinions on this. I will mention the people who are active or faced an issue related to this fix on this repo but this is public discussion.
@prh7 @hckr @eeliinaa @daqi @arturcarvalho @mcMickJuice @madhujahagirdar @persiasensei @shutchings @acemac @vasdee @Shvarts @hellooperatorpatchmethrough @jabidof @gaston-niglia @MadDeveloper @tobiasbueschel @tobiasbueschel @Cignite @msand @richardgirges

@prh7
Copy link

prh7 commented Sep 24, 2019

I have been following the RFC. It is a excellent work done. We have to distinguish the core timeline and the custom plugins i.e the custom layers. We also need to keep in mind the custom layers keeps the core timeline work as it is of today.

@Ilaiwi
Copy link
Collaborator Author

Ilaiwi commented Sep 24, 2019

@prh7 could you please elaborate on what do you mean by distinguishing the core?

@jabidof
Copy link

jabidof commented Sep 24, 2019

Hi and thanks for this great job.

This rowRenderer will allow virtualization? Thus improving rendering performance?

@prh7
Copy link

prh7 commented Sep 25, 2019

@Ilaiwi - The core means to keep the code library as it is of today. Extending the core with custom plugins/layers with least minimal changes being made to the core. This way I think we can avoid introducing new bugs when we implement our custom plugins/layers.

@Ilaiwi Ilaiwi pinned this issue Sep 25, 2019
@Ilaiwi
Copy link
Collaborator Author

Ilaiwi commented Sep 30, 2019

@jabidof there will be several performance bosts but it has to do more of trying to eliminate expensive calculations/renderers rather than through vertical rendereing.
for example this is profiling of a simple drag of an item horizontally.
Screen Shot 2019-09-30 at 3 09 49 PM
As you can see in the img. a drag horizontally affected every item on the screen and required a renderer. This is because internally all the items are rendererd in the same div and their absolute position is relative to the calendar so their top/left is changing every interaction.
With this new method. You are relative to your row rather than the whole calendar and all the existing items. This should limit renderers to only affect the row you are rendered in and only render the items inside the row. Or if moved vertically it should only render your group and then group effected without affecting any of the other groups.

@Ilaiwi
Copy link
Collaborator Author

Ilaiwi commented Sep 30, 2019

@prh7 Till now I am trying my best to keep the core code intact. But in reality, some code had to be changed (I hope to the better).
You can take a look at the compare here this is till WIP though
I am gonna release multiple alphas/betas for the feature and write docs on breaking changes and how to use the new feature.
I hope with the help of the community we can discover most bugs and squash them + write more unit tests

@arturcarvalho
Copy link
Contributor

I am gonna release multiple alphas/betas for the feature and write docs on breaking changes and how to use the new feature.
I hope with the help of the community we can discover most bugs and squash them + write more unit tests

If you need help testing, please let me know. And thanks for your efforts, the timeline is great!

@prh7
Copy link

prh7 commented Oct 1, 2019

I can help with testing. Thanks.😊

@jabidof
Copy link

jabidof commented Oct 4, 2019

@Ilaiwi
I'll take time to test and compare to current version.
Thanks for the job!

@Ilaiwi
Copy link
Collaborator Author

Ilaiwi commented Oct 30, 2019

@jabidof @prh7 @arturcarvalho
I've released beta here https://github.com/namespace-ee/react-calendar-timeline/releases/tag/0.27.0-beta
you can download it now with yarn add react-calendar-timeline@beta
for documentation checkout
https://github.com/namespace-ee/react-calendar-timeline/tree/develop

I will release a todo list soon with all the things needed to get this to master. Please let me know what do you think any try some of the new examples

@cosmith
Copy link

cosmith commented Nov 14, 2019

Hi @Ilaiwi, thanks for this great component and for the row renderer. We're using it to add drag-and-drop from another div in the page to add item to the timeline. The one thing we're not sure how to do is to be able to drop to a specific time ; i.e. right now we don't have any info as to where the item is in terms of date/time. So we place it at today/today+5 hours but I'm wondering if you have a solution in mind to this?

@msand
Copy link

msand commented Nov 15, 2019

@cosmith Check this: #156 (comment)

@Ilaiwi
Copy link
Collaborator Author

Ilaiwi commented Nov 15, 2019

@cosmith You can do that using timeline ref and the drop event. The rough idea is that it would figure out left from the event and from ref.scrollLeft and then use the helper https://github.com/namespace-ee/react-calendar-timeline/tree/develop#getdatefromleftoffsetpositionleft-number-number

@msand I've been over your solution so many times dude and it is great! This new feature aims to get a better solution using the library's public primitives and API. I haven't tried your solution with this new row renderer but it might be broken or maybe break in the near future. If you could help us to get this solution done using the react-dnd example we have for row render that would be really helpful
link https://codesandbox.io/s/timeline-demo-rowrenderer-dnd-from-outside-the-calendar-gz7ns

@cosmith
Copy link

cosmith commented Nov 15, 2019

Thanks @Ilaiwi , we'll try it out and let you know :)

@megasus
Copy link

megasus commented Nov 15, 2019

First of all, thanks @Ilaiwi. Your work is really appreciated.
Not sure if this is the right place to report bugs for this feature. I noticed that the canChangeGroup prop of an item has no effect when using rowRenderer. Every item can change groups when dragging. Tested it with 'demo-main'.

@Ilaiwi
Copy link
Collaborator Author

Ilaiwi commented Nov 15, 2019

@meengit Thanks for reporting this. I will take a look at the bug soon. And sure here works. I would appreciate any feedback I can get

@jeuhen
Copy link

jeuhen commented Feb 27, 2020

Hi @Ilaiwi, Thanks for all the work, I really like this component. Is there any update on this feature? I want to use it to have more control over row/item heights and, in the future, to drag and drop activities on the timeline. Do you have any idea when this will be released?

@Ilaiwi
Copy link
Collaborator Author

Ilaiwi commented Mar 2, 2020

@jeuhen Hey man. I am sorry but this doen't have a release date yet

@THPubs
Copy link

THPubs commented Mar 4, 2020

I think the group custom height is not working with rowRender yet right?

{
  id: 1,
  title: 'group 1',
  rightTitle: 'title in the right sidebar',
  stackItems?: true,
  height?: 30 <---- This one
}

@danielcruser
Copy link
Contributor

danielcruser commented Apr 24, 2020

@megasus I've found a fix for canChangeGroup having no effect. @Ilaiwi what would be the best way to contribute to this branch?

Lines 219 and 238 always use newGroupId, changing both of those lines to

const newGroupId = this.props.canChangeGroup ? e.dropzone.target.dataset.groupid  this.itemGroupId 

seems to bring back the correct behavior.

@Ilaiwi
Copy link
Collaborator Author

Ilaiwi commented Apr 28, 2020

@danielcruser target branch should be develop. And just submit a PR aginst it

@francescovenica
Copy link

the rowRenderer branch seems to be a lot behind the master, is this still in development? I'd love the idea to add a drag n'drop from outside

@sjustason
Copy link

We've tried to resolve conflicts from master and merged the rowrenderer work into a fork we have here: https://github.com/routific/react-calendar-timeline, there's a lot of great work from @Ilaiwi that we're trying to utilize; We're doing some testing now but aside from a few small issues we've fixed it largely seems stable. Once we've finished testing we'll happily open a PR to this original master with conflicts resolved for consideration.

@Ilaiwi
Copy link
Collaborator Author

Ilaiwi commented Jun 1, 2022

@sjustason could you email me at ahmad.ilaiwi@gmail.com ? would love to speak a bit about your guy's work

@jeuhen
Copy link

jeuhen commented Jun 17, 2022

@sjustason @Ilaiwi Great to see that this is still being worked on. Is there already a plan to bring this back to the main line or are you still testing/developing?

@magdalenaxm
Copy link

Is there any update for this feature? We already use it with an older version of the package, it would be great to use it in the latest version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request rowRenderer might solve the issue
Projects
None yet
Development

No branches or pull requests