Skip to content

Supply Appointment data to SchedulerSlotRenderEventArgs - #2050 #2154

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

Merged
merged 4 commits into from
May 27, 2025

Conversation

paulo-rico
Copy link
Contributor

As per request #2050, pass AppointmentData as a property in SchedulerSlotRenderEventArgs class.

Update demo site Year view to highlight this change.

Regards

Paul

@akorchev
Copy link
Collaborator

Hi @paulo-rico,

Thank you for the PR. My only concern is that AppointmentsInRange/Slot are being called multiple times for the same slot (once during layout and once in GetAttributesForSlot). In most cases the Appointments property in the SlotRenderEventArgs is not going to be used including all existing ones. It would still lead to overhead though. Two possible solutions come to mind:

  1. Store events per slot to not request them multiple times. Again seems redundant and memory consuming. Also prone to cache-miss/stale cache problems.
  2. Get Appointments on demand. Maybe the best solution. Pass a Func<IEnumerable<AppointmentData>> to GetSlotAttributes. Then either assign it directly to SlotRenderEventArgs or make the Appointments property call this func the first time it is used:
private Func<IEnumerable<AppointmentData>> getAppointments;
private IEnumerable<AppointmentData> appointments;

public IEnumerable<AppointmentData> Appointments => appointments ?? = getAppointments();

@paulo-rico
Copy link
Contributor Author

Hi @akorchev

Must say, I wasn't happy myself with calling the same function more than once per slot. Also, wasn't too sure about calling and storing the appointments. Just needed the nudge in the right direction regarding passing the function itself, so went with suggestion 2.

@paulo-rico paulo-rico changed the title Supply Appointment data to SchedulerSlotRenderEventArgs #2050 Supply Appointment data to SchedulerSlotRenderEventArgs - #2050 May 16, 2025
/// <summary>
/// List of appointments.
/// </summary>
public IEnumerable<AppointmentData> Appointments => appointments ?? (appointments = getAppointments());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use ??= here to make it shorter.

@@ -59,8 +59,8 @@
var excessCount = appointments.Count();
string classname = $"rz-slot-title {(day.Month != offSetMonth ? "rz-other-month" : "")} {(excessCount > 0 ? "rz-has-appointments" : "")} {(day == CurrentDate && month == CurrentMonth ? " rz-state-focused" : "")}".Trim();

<div @onclick=@(args => OnListClick(day, appointments)) @attributes=@Attributes(day, "rz-slot")>
<div class="@classname">
<div class="rz-slot" @onclick=@(args => OnListClick(day, appointments))>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks as a breaking change as it changes the location where Attributes are applied.

@paulo-rico
Copy link
Contributor Author

Hi @akorchev

You're right. It is a breaking change. I'll move the SlotRender code back where it was.

I do think we need to be able to style the day as in AppointmentRender. But it's not the same as AppointmentRender as the day represents the existence of appointments as opposed to a single appointment in all the other views.

It's calling out for it's own method - DayHighlightRender or something like that, that is defined only in RadzenYearView and will include all Appointments for the day, similar to what has just been added to SlotRenderEventArgs.

What do you think?

Regards
Paul

@akorchev
Copy link
Collaborator

How about using the SlotRender event as in other views? Right now all events are set at RadzenScheduler level and introducing event for a view would be a discrepancy.

@paulo-rico
Copy link
Contributor Author

I had used SlotRender, but in order to change the attributes on the "Day" as opposed to the "Slot", I had to move where the attributes were applied, and this was the breaking change.

Below is an image where the SlotRender is applied and rendered at the "Slot" level. As you can see, it's not ideal.

year-calendar-slot-render

It is still valid for a SlotRender to be applied for things like "Closed", "Today", "Holiday", e.t.c.

What do you think of having a DayNumberRender event at the Scheduler level? This would apply to Year, Planner, Timeline and Month views, and would style the day number in the slot.

@akorchev
Copy link
Collaborator

DayNumberRender is misleading as it won't apply for every single day number but just for day view. Do we need this event at all? Shouldn't we just use AppointmentRender for this case too (days that have appointments)?

@paulo-rico
Copy link
Contributor Author

DayNumberRender is misleading as it won't apply for every single day number but just for day view

It would be for Views I mentioned above -

What do you think of having a DayNumberRender event at the Scheduler level? This would apply to Year, Planner, Timeline and Month views, and would style the day number in the slot.

Shouldn't we just use AppointmentRender for this case too (days that have appointments)?

I have already looked into this, but AppointmentRender only passes the appointment that it references.

Looking at other libraries, maybe this is closer to what needs to be achieved.
Syncfusion Scheduler CellHeaderTemplate

Using a CellHeaderTemplate to override the "DayNumber" of the views. We could pass through the appointments for that day and the developer can react as they see fit. The context would be similar to SlotRenderEventArgs

@akorchev
Copy link
Collaborator

The original idea of this PR was "Supply Appointment data to SchedulerSlotRenderEventArgs". Let's stick to that.

@paulo-rico
Copy link
Contributor Author

Sorted (I think).

@akorchev akorchev merged commit ce0ce0f into radzenhq:master May 27, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants