maxkeppeler / sheets-compose-dialogs

✨ Enhancing Android UIs: A Jetpack Compose Library supporting a wide range of common use-cases with Material Design 3 Dialogs, Popups, and Bottom Sheets. ✨
https://maxkeppeler.github.io/sheets-compose-dialogs/
Apache License 2.0
807 stars 38 forks source link

[Crash] DateTime dialog adds an extra month and day to the initial value #52

Closed Nek-12 closed 1 year ago

Nek-12 commented 1 year ago

A datetime dialog defined as following

@Composable
fun datetimeDialog(
    title: String?,
    onSelectDate: (LocalDateTime) -> Unit,
    initial: LocalDateTime? = null, // kotlin datetime
    startWithTime: Boolean = false,
    maxYear: Int = MaxYear,
    minYear: Int = MinYear,
): UseCaseState {
    val state = rememberUseCaseState()
    DateTimeDialog(
        header = remember(title) { title?.let { Header.Default(it) } },
        state = state,
        config = DateTimeConfig(icons = LibIcons.Rounded, maxYear = maxYear, minYear = minYear),
        selection = remember(initial, startWithTime, onSelectDate) {
            DateTimeSelection.DateTime(
                selectedTime = initial?.time?.toJavaLocalTime(),
                selectedDate = initial?.date?.toJavaLocalDate(),
                startWithTime = startWithTime,
                onPositiveClick = { onSelectDate(it.toKotlinLocalDateTime()) }
            )
        }
    )

    return state
}

When shown, starts with month and day values incremented by one.

For example, a value of 5/9/23 (verified as being correct) when passed to the dialog, upon opening, shows that the selected date is "Jun 10 2023" which means that the day and month values are bigger than the actual ones by one. Clicking "OK" without changing anything updates values to correct ones, but reopening the dialog results in another increment, and this can be repeated indefinitely. After reaching month 12, the dialog displays the month value as unset. Clicking "OK" will crash the app.

Stacktrace

java.lang.NullPointerException
   at com.maxkeppeler.sheets.date_time.DateTimeState.onFinish(DateTimeState.kt:160)
   at com.maxkeppeler.sheets.date_time.DateTimeViewKt$DateTimeView$4$1.invoke(DateTimeView.kt:140)
   at com.maxkeppeler.sheets.date_time.DateTimeViewKt$DateTimeView$4$1.invoke(DateTimeView.kt:140)
   at com.maxkeppeker.sheets.core.views.ButtonsComponentKt$ButtonsComponent$1$3$1.invoke(ButtonsComponent.kt:97)
   at com.maxkeppeker.sheets.core.views.ButtonsComponentKt$ButtonsComponent$1$3$1.invoke(ButtonsComponent.kt:97)
   at androidx.compose.foundation.ClickablePointerInputNode$pointerInput$3.invoke-k-4lQ0M(Clickable.kt:781)
   at androidx.compose.foundation.ClickablePointerInputNode$pointerInput$3.invoke(Clickable.kt:775)
   at androidx.compose.foundation.gestures.TapGestureDetectorKt$detectTapAndPress$2$1.invokeSuspend(TapGestureDetector.kt:255)
   at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
   at kotlinx.coroutines.DispatchedTaskKt.resume(DispatchedTask.kt:177)
   at kotlinx.coroutines.DispatchedTaskKt.dispatch(DispatchedTask.kt:166)
   at kotlinx.coroutines.CancellableContinuationImpl.dispatchResume(CancellableContinuationImpl.kt:476)
   at kotlinx.coroutines.CancellableContinuationImpl.resumeImpl(CancellableContinuationImpl.kt:510)
   at kotlinx.coroutines.CancellableContinuationImpl.resumeImpl$default(CancellableContinuationImpl.kt:499)
   at kotlinx.coroutines.CancellableContinuationImpl.resumeWith(CancellableContinuationImpl.kt:368)
   at androidx.compose.ui.input.pointer.SuspendingPointerInputModifierNodeImpl$PointerEventHandlerCoroutine.offerPointerEvent(SuspendingPointerInputFilter.kt:672)
   at androidx.compose.ui.input.pointer.SuspendingPointerInputModifierNodeImpl.dispatchPointerEvent(SuspendingPointerInputFilter.kt:548)
   at androidx.compose.ui.input.pointer.SuspendingPointerInputModifierNodeImpl.onPointerEvent-H0pRuoY(SuspendingPointerInputFilter.kt:570)
   at androidx.compose.foundation.AbstractClickablePointerInputNode.onPointerEvent-H0pRuoY(Clickable.kt:739)

I suppose the error is that the month and day selection starts from 1, not zero :D

Nek-12 commented 1 year ago

You may want to start writing tests since the library has 380 stars now.

maxkeppeler commented 1 year ago

Tests exist for each module except for the date-time module due to a limitation. My free time is limited. Feel free to contribute changes. This could have been fixed quicker than writing this issue. When I have more free time, I will expand the current test cases and add date-time test.

Nek-12 commented 1 year ago

To be honest, I wanted to open a pr, but struggled to understand the codebase. It's quick for you, but I've spent about an hour trying to figure the source of an issue. But if I find a possible improvement, for example, regarding my other issue, I'll open a pr. Please don't take this as an accusation

maxkeppeler commented 1 year ago

There is only one location where the initial date and time is being used to calculate something. Your guess was correct, as the offset was wrong for selecting the corresponding value option by index, due to the day and month starting from 0.

Nek-12 commented 1 year ago

Can you please publish a version with this change included when you have free time?

Nek-12 commented 1 year ago

@maxkeppeler Friendly bump. No one can publish a version except you