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

Clicking backgroundEvent triggers onSelectSlot #2618

Closed
4 of 5 tasks
DianaLaa opened this issue Jul 11, 2024 · 6 comments
Closed
4 of 5 tasks

Clicking backgroundEvent triggers onSelectSlot #2618

DianaLaa opened this issue Jul 11, 2024 · 6 comments
Labels

Comments

@DianaLaa
Copy link

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
@geraldhoxha95
Copy link

geraldhoxha95 commented Jul 11, 2024

@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.

@cutterbl
Copy link
Collaborator

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.

@cutterbl
Copy link
Collaborator

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

@DianaLaa
Copy link
Author

DianaLaa commented Jul 12, 2024

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

@cutterbl
Copy link
Collaborator

@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.

@DianaLaa
Copy link
Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants