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

OnDayClick not triggered if there are events for the week on other days #96

Closed
scottlovegrove opened this issue Jul 17, 2023 · 4 comments · Fixed by #109
Closed

OnDayClick not triggered if there are events for the week on other days #96

scottlovegrove opened this issue Jul 17, 2023 · 4 comments · Fixed by #109
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@scottlovegrove
Copy link
Contributor

Describe the bug
So this could be quite a problematic bug I'm afraid.

If you have events for a day, when those events are rendered, if you try clicking on a day that doesn't have an event, the onDayClick eventHandler isn't fired if you clicked parrallel to the events. If you click lower down in the day below where the events finish getting rendered, then the onDayClick eventHandler is fired.

To Reproduce
Pull down my branch for this commit scottlovegrove@8604a69, there is a story called OnDayClickBug

image

This is made even worse on mobile because you may not have the abundance of space seen in the screenshot, effectively making the onDayClick event completely useless.

Expected behavior
The onDayClick event should get triggered when it's clicked (I will concede it shouldn't if you have actually clicked on an event).

Additional context
As it happens, I've managed to get around this issue in my own app because I don't care if the user clicks on an actual event, so I've just added this to my css

.schedulely .event-week-layout {
    pointer-events: none;
}
@scottlovegrove scottlovegrove added the bug Something isn't working label Jul 17, 2023
@bruceharrison1984
Copy link
Owner

Yes, that one will be tricky. The way the grid is overlayed on top of the calendar means that onClick handlers won't propagate through directly to the day component.

Hopefully I can get around to this quickly. I just took on a new client so I don't have a ton of bandwidth ATM.

@scottlovegrove
Copy link
Contributor Author

Hopefully I can get around to this quickly. I just took on a new client so I don't have a ton of bandwidth ATM.

From my perspective, that's fine, as mentioned, I don't need users to be able to click on individual events, so I just bypass the events row completely :) I was just logging the bug while I thought about it because it will likely affect some users at some point.

@bruceharrison1984
Copy link
Owner

bruceharrison1984 commented Jul 17, 2023

Well I appreciate the issue submissions! I'm wondering if some CSS trickery can get around this, or potentially merge the event grid with the day of week grid. This wasn't completely surprising, because I knew there were some stacking issues related to this.

@bruceharrison1984 bruceharrison1984 added the help wanted Extra attention is needed label Jul 30, 2023
@bruceharrison1984 bruceharrison1984 linked a pull request Sep 11, 2023 that will close this issue
@bruceharrison1984
Copy link
Owner

This was easily fixed by disabling pointer-events on the grid itself, and then re-enabling them for all child divs. This could cause issues if someone tries to using something other than a div to contain a custom event, but the answer would then be to just use a div.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants