jquense/react-big-calendar

Clicking backgroundEvent triggers onSelectSlot

DianaLaa opened this issue · 8 comments

Check that this is really a bug

  • I confirm

Reproduction link

https://codesandbox.io/p/sandbox/react-big-calendar-example-forked-vpt5d8

Bug description

Reproduction scenario:

  • Click BackgroundEvent
  • Expected: onSelectEvent is called
  • Actual: onSelectSlot is called, then onSelectEvent is called

This occurs when the calendar has prop selectable or selectable='ignoreEvents'

Expected Behavior

When clicking a backgroundEvent, do not trigger onSelectSlot.

Actual Behavior

When clicking backgroundEvent, first onSelectSlot is called, then onSelectEvent is called

react-big-calendar version

1.12.2

React version

18.3.1

Platform/Target and Browser Versions

Chrome latest

Validations

  • Read the docs.
  • Check that there isn't already an issue that request the same feature to avoid creating a duplicate.
  • Make sure this is a react-big-calendar issue and not an implementation issue

Would you like to open a PR for this bug?

  • I'm willing to open a PR

@DianaLaa Have a fix here: #2619

Root of the issue is that, when determining if onSelectSlot should be triggered, it uses the isEvent function, which only considers a node with class rbc-event as an event, and not rbc-background-event

Although I am not sure if this is actually a bug or working as expected, as it seems that backgroundEvents are meant to be overlayed on top of, which might mean its expected to trigger onEventSlot. @jquense or @cutterbl might need to chime in here.

This behavior has been in place since background events were introduces. Because there are people that use this we cannot change it to ignore it. However we did make changes to the onSelectEvent to provide details to differentiate if you need to ignore. As the documentation on that link states

Clicking on a backgroundEvent will also fire the onSelectEvent callback. It will also receive the backgroundEvent as the event object, but contain a isBackgroundEvent property set to true. This will allow you to distinguish between a background event and a regular event within your onSelectEvent handler.

@geraldhoxha95 And yes, onSelectSlot should fire, as events would overlay on top of background events. (such as using them to show availability time frames)

Hello, thank you for your investigation and quick replies. I understand your hesitation in introducing a breaking change.

I've digged some deeper into our production code and I have some follow-up questions I hope you're willing to provide a little support with:

  • Is there some way I can detect in my onSelectSlot handler that a backgroundEvent was clicked and that I can abort the handler because onSelectEvent is going to come to pass, too? (I.e. don't show a "Create new event" dialog, because the "Background event details" dialog needs to be shown)
  • Is there a scenario where onSelectEvent is not triggered after onSelectSlot was handled? Is there a check or detaching of a handler somewhere? This seems to happen in our production code. However, in the codesandbox I created for this ticket (see above) both handlers are called. I've tried to look into react-big-calendar source code to find this answer but I'm not familiar with it so I haven't wrapped my head around how it all ties together. --> Found it, see https://codesandbox.io/p/sandbox/onselectslot-should-not-be-called-forked-p9lhd3

Lastly, some curiosity: Could you help me understand with an example what is the use case for triggering both onSelectSlot and onSelectEvent when the user clicks a background event? (Not click-and-drag over a background event, not a background event with a regular event on top that is clicked, but just a regular click on a free-floating background event. In that case I would expect only onSelectEvent since the user purposefully clicked.)

Thanks for your time.

Kind regards,
Diana

@DianaLaa I can't give you a lot of details unfortunately, as all of that functionality was written into RBC long ago. I can tell you that, personally, the product I work on uses timers and cancellation (like you see in many of our drag and drop examples in our documentation) to attempt to determine what the user's true intentions are, and only call the event it needs to in the end. My hope is that our next major version (which will be a complete rewrite and major breaking change) will address these sort of issues, but we do not have a solid timeline for that as yet.

@cutterbl Thank you for your reply and suggestion. I've added a check to our onSelectSlot method that first searches through the list of backgroundEvents to see if any of them was clicked. If so, the handler is exited so that onSelectEvent is triggered.

Hi, I am encountering the same issue here because I am using both backgroundEvents and events, my logic was to use backgroundEvents to disable slots from being selected, while events would be actual reserved slots.

I was hoping onSelectSlot would not fire when clicking on backgroundEvents.

I noticed that we could pass ignoreEvents to selectable, I assumed it would not trigger onSelectSlot when clicking a background event, but unfortunately it doesn't and was wondering why.