react-native-calendars icon indicating copy to clipboard operation
react-native-calendars copied to clipboard

In AgendaList provide a prop to always render empty items for all days rather than iterate over an array for empty days

Open shamilovtim opened this issue 6 years ago โ€ข 14 comments

Description

Currently, empty days must be provided with an empty array like this {'yyyy-mm-dd':[]}. After iterating through those empty days, the AgendaList renders the fragment returned from renderEmptyDate. For devs who want a design that has empty dates always rendered, this uses a ton of processing power because they must then create a function where they provide a year of empty dates. For these developers, it would be beneficial to provide a prop alwaysRenderEmptyDates: boolean. This prop will always render an empty item, rather than looping through an array in order to get the empty items, if a day is empty, it will render an empty day automatically. In theory this should save CPU cycles and increase speed.

Here is an example where I'm always rendering empty days: Screen Shot 2019-10-14 at 3 01 30 PM

shamilovtim avatar Oct 14 '19 19:10 shamilovtim

It's a great feature but I am not sure about the implementation.

Should we render a year of empty dates and then fill up only the sessions? Not sure how beneficial it is in terms of performance.

Should we get the marked dates and then calculate what is missing in between?

We need to think about solid solution.

chenei avatar Jun 04 '20 13:06 chenei

@chenei The performance gain should be from not having to loop through an array. If there is no item, return the empty component.

Should we get the marked dates and then calculate what is missing in between?

We should render an empty for all dates on the calendar that are available. That means if a calendar is 3 years, always show an empty for 3 years.

shamilovtim avatar Jun 05 '20 15:06 shamilovtim

I explained this above but there's a difference between actively looping for every single array entry being empty which is (O)n and rendering an empty when there's no array entry which is rather than (O)n costs nothing.

shamilovtim avatar Jun 05 '20 15:06 shamilovtim

Since we have a date attached to every cell, we need to fill the list with empty dates. I donโ€™t know how you are planning to do that in O(1), but you do need to iterate through all the dates in range of 3 years and for each date render an empty cell. This is definitely O(n) so regarding performance, this is the same. I do understand that from user perspective itโ€™s easier to pass a Boolean instead of array with dates.

Letโ€™s render 5 years since 365 * 5 is basically O(1).

chenei avatar Jun 05 '20 15:06 chenei

@chenei Thanks! Maybe I'm misunderstanding but let's say you have an array of entries that aren't empty: ['2018-07-01', '2019-07-01', '2020-07-01'] you loop through the array and render the real items, you've looped through 3 items. When a real item doesn't exist for dates between the start and the end, you render an empty item. This isn't testing 3 years worth of items, this is testing 3 items or else rendering an empty. If, instead, you had to fill the array with 3 years of items, then you would have to have 1095 items most of those empty that you had to loop through. Does that make sense?

shamilovtim avatar Jun 05 '20 15:06 shamilovtim

@shamilovtim I think we still have misunderstandings :)

Ok let's say we have ['2018-07-01', '2019-07-01']. When you say When a real item doesn't exist for dates between the start and the end, you render an empty item. it actually means that we need to go over all the dates between 2018 to 2019 and render empty cells, so we need to iterate through 363 items. There is no other way to do that, you need to figure out which dates are missing in between and fill them with an empty cell.

This is the same performance as passing an array of 365 items or maybe worse because you need to calculate which days are missing using Moment or something. A better solution is to fill all the Agenda with empty cells and then go over the "Real Items" and fill them in those empty cells.

Overall, rendering 1095 items is not effecting performance so much.

chenei avatar Jun 05 '20 17:06 chenei

Gotcha. I guess I assumed that you wouldn't be looping through an array of dates to begin with but rather seeing if the dates exist inside of an array. I think your solution is much better, always populating empty items but only replacing them if a date exists.

shamilovtim avatar Jun 05 '20 17:06 shamilovtim

Now that you have a maintainer do you believe there is any performance increase to be found here? @chenei @emilisb

I believe providing a default empty component which always renders and is then replaced with a day component on days with events is much more performant than the current approach because for instance it would:

  1. eliminate our useCallback(()=> ...) function that creates 365 empty items.
  2. stop us from looping through an array 365 times
  3. eliminate lots of useless lines of code
  4. simplify the props and strategy of the calendar with regards to providing empty items

that being said, it would be a large change and I understand that comes with some inconvenience.

shamilovtim avatar Dec 02 '20 16:12 shamilovtim

Based on my experience investigating WixApp performance issues, such calculations do not make a large impact in performance as long as they are performed once and not on every single render call. The rendering and wasted rerenders is what slows down React Native apps most of the time (this library included). This change would probably not make very noticeable difference (it may save 0.5-1ms if I understand the situation correctly), but it would definitely make our API more user friendly.

We are currently focusing on performance improvements at Wix so we will consider this ๐Ÿ‘

emilisb avatar Dec 04 '20 08:12 emilisb

@emilisb I think it's also a little more difficult to use to have to generate the empty dates beforehand instead of just passing the non empty dates and passing a component to a func prop called renderEmptyItems. Similar comments on an older issue I found:

  • https://github.com/wix/react-native-calendars/issues/776#issuecomment-477310944
  • https://github.com/wix/react-native-calendars/issues/15#issuecomment-302137947

KrisLau avatar Mar 15 '22 15:03 KrisLau

Yeah the code I had to write to do this was insane. I would love to delete it and just get empty days if I want empty days.

shamilovtim avatar Mar 15 '22 16:03 shamilovtim

Yeah the code I had to write to do this was insane. I would love to delete it and just get empty days if I want empty days.

@shamilovtim Agreed, I would love to add empty day placeholders because personally, I think it is a little confusing to see dates that are weeks or months apart next to each other. Also it would just be nice for users to just have the peace of mind of seeing an empty date

KrisLau avatar Mar 15 '22 16:03 KrisLau

This is really needed

Polad10 avatar Sep 26 '23 15:09 Polad10

any solution on this ?looking for help

AmaanSayyed avatar May 24 '25 12:05 AmaanSayyed