Skip to content
This repository has been archived by the owner on Sep 8, 2020. It is now read-only.

Fix issue with event rendering #353

Closed
wants to merge 2 commits into from

Conversation

matiascaputti
Copy link

Fix event issue that make them disappear if switch with previous/next or change date from calendar.

@flick36
Copy link

flick36 commented Apr 12, 2016

#346

@martin-langhoff
Copy link
Contributor

My initial reaction to this is so-so. The standard behavior of FC is to default to ephemeral events. angular-ui-calendar is wrapping FC, so overriding FC default behaviors needs to be done with due care.

In my code using angular-ui-calendar, I set sticky in the draggable object "event" properties, or register an eventReceive() cb and apply the fixup there.

In other words, sticky definitely deserves an FAQ, but I'm not sure it deserves an override.

@arshaw - I am interested in your take on why FC defaults to ephemeral (sticky: false) for 'received' events from a drag&drop action.

@arshaw
Copy link

arshaw commented Apr 13, 2016

because usually upon drop of an event, a write call is made to the backend DB, and if the user navigates away from the current view and back again, the end-user will receive the newly created event from the DB, and if it was sticky when it was added, they will see two. that was my thought process

@martin-langhoff
Copy link
Contributor

@arshaw - thanks!

@maticaputti , @flick36 -- at this time I do not think it is a good idea to merge this. Reasons:

  • ui-calendar is a wrapper around FC, and should favor following the FC defaults and behaviors. This default in FC fits one programming pattern as @arshaw points out. Might be surprising for folks using other patterns (as it was for @maticaputti and for me when I used it initially) but that's material for a FAQ.
  • This would be a breaking change; affecting users who have been relying on the sticky:false behavior. Breaking changes require major reasons behind them...

I don't like to see a nice convenient patch go to waste but...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants