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

Dynamic height on CalendarListItem

Open freirepaulo opened this issue 2 years ago • 9 comments

Description

Issue related: Doesn't exist a feature of dynamic height on CalendarListItem #2209

I've been using the react-native-calendars in my project to show the list of one year in the calendar list component. Unfortully until the beginner I prepared if the first mount didn't start on the first day, for example, on April 15, the height of the calendar will be the same then the end of the mouth will be empty (example above)

Propose Solution

for me, the CalendarListItem height needs to be dynamic based on the initial date of the month. So if the month is full like starting on the 1st the height will be full but if the month started on the 15th the height need to be half of the full month.

I made a little change to the code and works for me, however, I would like to see with this PR if I need to consider more use cases of that and which improvement I need to do for this PR to be merged.

here we have these examples of the same screen, one with the current code and the other with my code.


To give a breath explanation of the code, I'm basically using the index of the renderItem to look for the first element of the calendar. after that, I take how many days this month has in total, and then after already knowing the initial date I can subtract the initial date of the total days to have the latest days of the month. With that value, I can divide it by 7 (a week) to get how many lines the calendar will have and then multiply by the CALENDAR_LINE_HEIGHT to get the height of this month.

freirepaulo avatar Apr 11 '23 12:04 freirepaulo

I need this feature so I tried this change out but it didn't work for my use case. I'm using a CalendarList that needs to scroll over the maximum range, plus a custom day component. I poked around in the code and haven't quite been able to get it working but I think an approach like this might work.

Define a getCalendarHeight function which determines the number of weeks in a month, then computes the calendar height based on a passed-in dayHeight prop + a HEADER_HEIGHT constant (although this would break if the header style is overridden, so headerHeight should probably be an optional prop as well). e.g.:

    const getCalendarHeight = useCallback((month, year) => {
        if (calendarHeightDynamic) {
            const numDays = new Date(year, month, 0).getDate()
            const firstDay = new Date(year, month - 1, 1).getDay()
            const lastDay = new Date(year, month - 1, numDays).getDay()

            let numWeeks = Math.floor((numDays + firstDay) / 7) // Number of full weeks
            if (lastDay < 6) { // If the last day is not a Sunday, count the partial week
                numWeeks += 1
            }
            return (dayHeight * numWeeks) + HEADER_HEIGHT
        }
        return calendarHeight
    }, [])

That's pretty straightforward — the other part will be calculating the correct offsets in getItemLayout, scrollToDay, and scrollToMonth. I think this could be accomplished by constructing a map with keys in the form of ${month}-${year} and heights as values, then using that to compute the offset or scroll range for a given month.

I have to move on to something else for now but if no one else gets to it I'll try to implement later.

stevengoldberg avatar Jun 26 '23 00:06 stevengoldberg

@stevengoldberg, it's great to know that this issue is affecting others as well because until now, I hadn't seen any open issues regarding it.

I believe it's important for this solution to work for all use cases. In my case, it works and meets my needs, but I found your comment about the HEADER_HEIGHT size interesting.

I will take into consideration the things you mentioned when making a new commit, but I'm also looking forward to seeing your implementation so that we can create a solution together that satisfies the Wix team as well.

freirepaulo avatar Jun 26 '23 10:06 freirepaulo

Those last commits were made to implement the changes suggested by @stevengoldberg . They involve considering the day and header heights to calculate the dynamic height of the calendar.

I need to add new props to the calendar list component: dayHeight and headerHeight. These props will provide information about the component because we are unable to retrieve the heights from the dayComponent prop or the customHeader prop. By using these new props and setting default values for them, we can ensure that nothing breaks if the developer uses the default day and header components. With this approach, the developer will need to create two constants: BASIC_DAY_HEIGHT and HEADER_HEIGHT, which will set the default height for these components (I have used the size that we are already using).

Note: I tried to obtain the heights dynamically from the custom components and even from the themes, but it was not possible. Therefore, we need to find a way to set a default value for the day height and header height because, if the developer uses the default components, these values will be undefined.

I believe this approach covers all use cases and is not a breaking change, so @Inbal-Tish can review it again to see if the merge is now possible.

Note: @stevengoldberg, I think the second part of your suggestion is more of a separate issue. We are waiting for @Inbal-Tish's response regarding this new code and will reevaluate if those additional changes make sense for this pull request.

freirepaulo avatar Jul 13 '23 17:07 freirepaulo

Those commits look good to me, but I think what's missing is handling calendarSize, which is used in getItemLayout and the scrollTo methods. Right now calendarSize is still static, so I don't see how this can work if the actual calendar size is dynamic.

stevengoldberg avatar Jul 13 '23 17:07 stevengoldberg

@stevengoldberg, I know it's still missing, but I'm waiting for a response from someone at Wix to confirm that these changes are approved so that I can look into the scrolling. However, if I have some time this week, I'll work on it. And, of course, feel free to implement this feature as well.

freirepaulo avatar Jul 31 '23 10:07 freirepaulo

I also experience inconvenience with current behavior. Static height can only benefit the horizontal calendars, despite static width that may benefit both, since the week length is not dynamic and can be calculated in advance.

This is how it looks for me: Screenshot 2023-08-30 at 21 23 33

For those who wanders trying to resolve it, I'd recommend using patch-package, just grab the code from this pull request into the node_modules and create a patch

kioltk avatar Aug 30 '23 18:08 kioltk

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Dec 15 '23 05:12 stale[bot]

@kioltk Yes bro, I solved my problem using the patch-package, but the best solution will be to resolve this internally in the library, unfortunately, the Wix team doesn't give us attention to be able to merge those changes.

freirepaulo avatar Jan 05 '24 14:01 freirepaulo

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Apr 22 '24 08:04 stale[bot]