Resize Month Events and Resize Week/Day Events on Both Ends #578

arecvlohe · 2017-10-05T14:06:49Z

Description

This PR adds the ability to resize month events as well as resize week/day events on both ends, top and bottom. I wasn't able to get this to perform correctly on hover which I suppose is ideal. We might find inspiration from #577.

This is a long standing issue so it would be good to see some movement on this.

Also, I think this could be a good first implementation that is refined on going forward.

Screenshots

Month

Week

Issues

#382

PRs

#577, #541, #406

Summary: There were errors in my branch because importing the .default was not pulling in the correct default whereas when I removed it, it seemed to work fine. This might be some kind of change in React 15.6 for how default modules are imported/required.

Summary: For <EventCell /> to know that it can be resized I am passing down the resize prop to that component. This is because, unlike in the previous implementation, I am not passing down what would be the eventWrapper from the dragAndDrop addon. I didn't know how to pass down multiple components based on whether the calendar was resizable. So I went this route. Sorry, not sorry.

Summary: I created <ResizableEventLR /> to handle resizing events in the month view. If <EventCell resizable /> then it will render the resizable component. I added styles so that I could see what is going on. There is a red bg. Sorry, not sorry. There is no functionality at the moment but I started building it out. There is certainly more to be worked on here.

Summary: Updated the resize function to be aware of the end as well as the start time. This is when there are two dnd handlers on an event at once. I renamed the component to be more semantic with its usage. Styles were correspondingly updated.

Summary: I didn't realize the conent should be aligned left.

Summary: Refactored components to look a bit more readable.

Summary: Refactored <ResizableEvent /> and added the ability to resize the top component.

Summary: Refactored the if/else statement into a switch statement.

Summary: I deleted these chanages thinking they were old. My mistake!

Summary: Remove unused code.

arecvlohe · 2017-10-05T16:43:46Z

Actually, I think I will keep this open

svanetten1976 · 2017-10-17T16:55:41Z

@arecvlohe @jnmandal any idea if this excellent feature you guys added is going to get merged into the project?

arecvlohe · 2017-10-17T18:17:02Z

@svanetten1976 I don't have a good idea. @jquense might know better than me. I don't really know what/how things get vetted and merged.

jnmandal

💯💯💯 thanks for picking this up @arecvlohe

jquense · 2017-10-18T12:21:36Z

src/EventCell.js

@@ -49,6 +50,10 @@ class EventCell extends React.Component {
    if (eventPropGetter)
      var { style, className: xClassName } = eventPropGetter(event, start, end, selected);

+    if (this.props.resizable) {


These bits should be encapsulated in the addon code, the core library should not require things from /addons (so that they are optional)

I see what you mean here. In our implementation at work I ended up removing this.

jquense · 2017-10-18T12:22:43Z

src/DayColumn.js

      , messages
-      , eventComponent
-      , eventTimeRangeFormat


Seems like a rebase error

Actually this is because of a fix I made in #406. eventComponent was being destructured incorrectly in the DayColumn component's renderEvents function (so it was always undefined). We need to be able to specify custom eventComponent for the addOn to work properly.. But its possible this has since been fixed some other way.

Instead of using event component. I updated the addon to pass the ResizeEvent to components: { day: { event }, week: { event } } as opposed to creating a new prop inside of components. I think this is how the library is intended to be used.

jquense · 2017-10-18T12:23:54Z

examples/App.js

@@ -101,7 +102,7 @@ const Example = React.createClass({
                <strong><i className='fa fa-code'/>{' View example source code'}</strong>
              </a>
            </div>
-            <Current className='demo' />
+           <Current className='demo' />


It looks like a space was removed.

can we fix it?

You mean put the space back in?

jquense · 2017-10-18T12:24:32Z

examples/demos/dnd.js

+    // if we want to update the event while dragging we need to find the event
+    // by an identifier. here we use the title and the start time, but depending
+    // on use case you may want have a truly unique id attribute in the event
+    const nextEvents = events.map(existingEvent => {


Why doesn't indexOf work here?

@jquense this was due to a quirk in react-dnd, copying my explanation from #406

Since only the original event object is accessible through react-dnd... In the onEventResize callback you will not be able to use Array.prototype.indexOf for finding the event again -- unless you continue to mutate the original event and splice it into the cloned events array... but I'm not sure if we would want to recommend that practice

Ideally in the next iteration of the callback the new event will be there? Why can't we get react dnd to update the thing it's referencing. I don't love this logic is fairly flakey even for an example and likely to lead folks to copying it in their own code with potential issues. Last ditch we should at least add an id to events in the example and use that vs title and date instances

tobiasandersen · 2018-01-10T14:33:12Z

👍 Awesome! But there's quite a lot of noise here. Are all those commits and line changes necessary/relevant to this PR? cc @jquense

arecvlohe · 2018-01-10T17:10:29Z

@tobiasandersen Are you saying to rebase or remove things?

tobiasandersen · 2018-01-10T17:41:34Z

Rebase (or squash merge) :) I saw now that the heavy line changes was due the example bundle, which I guess makes sense!

arecvlohe · 2018-01-10T17:43:47Z

I will rebase some changes to remove some fluff from the commit history @tobiasandersen

arecvlohe · 2018-01-10T22:58:43Z

@tobiasandersen Doing a rebase is too insane, at least for me, with all the commits that have been merged over the past couple days. But I also think I missed what you were saying. Did you mean I can merge, just choose either the squash merge or rebase merge option?

tobiasandersen · 2018-01-10T23:43:49Z

Squash and merge would combine all your 47 commits into one. I usually prefer having 1 (or a few maximum) commits per PR. It keeps the history clean, and makes it easier when going back to find when and why things were introduced. However, it also removes the actual history. And I see that you've been a few people working on this – squashing would make it look like you wrote all of it.

I'd prefer a squash over the 47 commits if I were to choose from the two. But it's @jquense's project, so I think it's better if he decides what's best :)

arecvlohe · 2018-01-11T13:59:29Z

@tobiasandersen I hear you. Let's see what @jquense says...

jquense · 2018-01-11T14:20:15Z

@arecvlohe You do whatever makes your life a bit easier to handle the rebase. Generally I think it's worth leaving the branch unsquashed and then squashing via the github butten whe nwe merge so that master ends up with one commit, and the original history is maintained. In this case tho i can imagine that the rebase is super painful without squashing so do what makes it's easier for you. We'll be sure to shout to out everyone who contributed to this PR

…eature/events-resize

arecvlohe · 2018-01-11T17:48:11Z

I ran into another hiccup. After merging in master, in the DnD example I am not able to drag and drop events in the week and day view. However, month view works fine.

I will have to dig into this at some point but there is a lot to dig so any help is appreciated.

arecvlohe · 2018-01-11T17:48:29Z

@tobiasandersen @AlexMarvelo @jquense ^^^

tobiasandersen · 2018-01-11T21:24:44Z

I wouldn't be surprised if it's due to my latest PR (although I did test the DnD example before merging). I'll have look and get back.

arecvlohe · 2018-01-11T22:21:15Z

I will take a look this evening and see what I can find out

tobiasandersen · 2018-01-11T22:36:32Z

Cool! If it's anything from my last PR, it's probably this extra div that I added: https://github.com/intljusticemission/react-big-calendar/blob/0b2c2347efa8fe4fb77f7931634de9a8ee1362ac/src/DayColumn.js#L132.

If you don't find the problem I'll have a check when I wake up tomorrow!

arecvlohe · 2018-01-12T05:10:40Z

@tobiasandersen From what I can tell it looks like the event container is the culprit. Unfortunately in react-dnd there is not a way to drop on a div and have the drop be handled by a div underneath. So basically it just covers where it can be dropped and that is why nothing is happening. One way to maybe get around this is to set the z-index of the date cells to > 1 when there is a drag event.

tobiasandersen · 2018-01-12T09:31:13Z

Sorry about that, should have tested it more than I did. I'm thinking pointer-events: none on that div should be enough?

arecvlohe · 2018-01-12T13:34:46Z

I will check that out and see

arecvlohe · 2018-01-12T13:37:22Z

It works! I will wait for a successful build to complete first and then move forward with merging into master

arecvlohe · 2018-01-12T14:17:26Z

It is done 😄

tobiasandersen · 2018-01-12T14:20:17Z

Awesome! Great work! 🎉

jadczyk · 2018-01-14T17:01:47Z

Hi, after your merge it seems you cannot select events in weekly view anymore when not using drag'n'drop. You added pointer-events: none; to .rbc-events-container but didn't add pointer-events: all; to .rbc-event

arecvlohe · 2018-01-14T20:03:41Z

Good catch @jadczyk!

arecvlohe · 2018-01-14T20:04:42Z

@jadczyk Mind creating a PR to fix this?

jadczyk · 2018-01-14T20:18:46Z

Just did #690

ehahn9 · 2018-02-03T21:10:05Z

Just started using the resize/DnD stuff - thanks @arecvlohe & others - hugely useful!

tl;dr: I'm looking for advice on how to best marry DnD with custom components.

When creating the underling calendar, it looks like the addon's withDragAndDrop HOC overwrites a bunch of component: props to add DnD. Alas, I have a custom Event component in day/week/month views (and in a custom view), so this is a bit of a challenge.

Would something like this make sense:

   const DnDCalendar = withDragAndDrop(BigCalendar)
   <DnDCalendar
     components={{
       week: {
         event: myCustomEventComponent
       }
     }}
   />

becomes instead:

   <DnDCalendar
     components={{
       week: {
         event: withDragAndDrop.customComponent(myCustomEventComponent)
       }
     }}
   />

The new HOC would wrap the user's components with one based on the current ResizableEvent (yes, there would have to be a month view variant, too).

I'm keen to get folks' other ideas/approaches (or validation of this one). I'm probably misunderstanding the DnD philosophy, but it was a surprise to see examples/dnd basically overwrite custom component props.

thx for any guidance/advice.

alxmiron · 2018-02-04T11:30:19Z

I think that no more additional work should be required to enable DnD except wrapping the calendar with withDragAndDrop. It's the best practice. I will take a look, what can be done. I see, that DnD addon does not really take care of custom event renderer now.

jnmandal and others added 21 commits May 20, 2017 12:20

Fix incorrect destructuring of eventComponent in DayColumn#renderEvents

d498d63

Pass through an event component with a resize anchor

06e8eb4

Make resize anchor draggable

dbeb301

Make resize anchor droppable

0d2208a

Use isOver prop to resize

587cd1c

Rename and refactor

ec059ae

Correct alignment of entire anchor and not just icon

b0c2c74

Differentiate between drag and drop callback fires

e44034b

Add example code for using the resizable event feature

c84f2a4

Rename ResizableEventComponent -> ResizableEvent

7d3c6df

Update Examples and Fix Errors

7d8f922

Summary: There were errors in my branch because importing the .default was not pulling in the correct default whereas when I removed it, it seemed to work fine. This might be some kind of change in React 15.6 for how default modules are imported/required.

Update Styles

e985954

Summary: I didn't realize the conent should be aligned left.

Refactor Components

b37ecd1

Summary: Refactored components to look a bit more readable.

Add Top Drag and Refactor Component

aae9bc5

Summary: Refactored <ResizableEvent /> and added the ability to resize the top component.

Refactor If/Else Statement

d16f5d9

Summary: Refactored the if/else statement into a switch statement.

Merge Upstream/Master in Local

81b3e50

Add Back Changes from Master

bf9a0e6

Summary: I deleted these chanages thinking they were old. My mistake!

Remove Unused Code

c8687ce

Summary: Remove unused code.

arecvlohe closed this Oct 5, 2017

arecvlohe reopened this Oct 5, 2017

arecvlohe mentioned this pull request Oct 5, 2017

Feature request: resize events #577

Closed

jnmandal approved these changes Oct 17, 2017

View reviewed changes

jnmandal mentioned this pull request Oct 17, 2017

Resizable Events #406

Closed

jquense reviewed Oct 18, 2017

View reviewed changes

Merge https://github.com/intljusticemission/react-big-calendar into f…

3c3e79c

…eature/events-resize

tobiasandersen mentioned this pull request Jan 12, 2018

Release a new version with changes since 0.17 ? #666

Closed

fix: week/day resize events

9d78b00

docs: update docs with resizable prop

90843a5

arecvlohe merged commit 546c4da into jquense:master Jan 12, 2018

Resize Month Events and Resize Week/Day Events on Both Ends #578

Resize Month Events and Resize Week/Day Events on Both Ends #578

Conversation

arecvlohe commented Oct 5, 2017 • edited Loading

Description

Screenshots

Month

Week

Issues

PRs

arecvlohe commented Oct 5, 2017

svanetten1976 commented Oct 17, 2017

arecvlohe commented Oct 17, 2017

jnmandal left a comment

Choose a reason for hiding this comment

jquense Oct 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jquense Oct 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasandersen commented Jan 10, 2018

arecvlohe commented Jan 10, 2018

tobiasandersen commented Jan 10, 2018

arecvlohe commented Jan 10, 2018

arecvlohe commented Jan 10, 2018

tobiasandersen commented Jan 10, 2018

arecvlohe commented Jan 11, 2018

jquense commented Jan 11, 2018

arecvlohe commented Jan 11, 2018

arecvlohe commented Jan 11, 2018

tobiasandersen commented Jan 11, 2018

arecvlohe commented Jan 11, 2018

tobiasandersen commented Jan 11, 2018

arecvlohe commented Jan 12, 2018

tobiasandersen commented Jan 12, 2018

arecvlohe commented Jan 12, 2018

arecvlohe commented Jan 12, 2018

arecvlohe commented Jan 12, 2018

tobiasandersen commented Jan 12, 2018

jadczyk commented Jan 14, 2018

arecvlohe commented Jan 14, 2018

arecvlohe commented Jan 14, 2018

jadczyk commented Jan 14, 2018

ehahn9 commented Feb 3, 2018

alxmiron commented Feb 4, 2018

arecvlohe commented Oct 5, 2017 •

edited

Loading

jquense Oct 18, 2017 •

edited

Loading

jquense Oct 18, 2017 •

edited

Loading