Skip to content
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

Week Calendar scroll to past skips not scrolls #567

Open
akrulec opened this issue Jul 25, 2024 · 15 comments
Open

Week Calendar scroll to past skips not scrolls #567

akrulec opened this issue Jul 25, 2024 · 15 comments

Comments

@akrulec
Copy link

akrulec commented Jul 25, 2024

Library information:

  • Version: [2.6.0-beta02]
  • View or Compose module: Compose

Describe the bug**

I recently migrated from Views to Compose, so I just went with the newest library (but I also tried 2.5.2 which is the last stable version). When I try to scroll, it only scrolls forwards, but not backwards. Is that a setting that I accidentally set somewhere, or is that a bug? Thank you

Expected behavior (if applicable)

Ability to scroll forwards and backwards.

Screenshots? (if applicable)

Attached screen recording.

Screen_Recording_20240724_182411_trainwell.mp4

Additional information

    val state = rememberWeekCalendarState(
        startDate = startDate,
        endDate = endDate,
        firstVisibleWeekDate = currentDate,
        firstDayOfWeek = firstDayOfWeek)

    var firstDateOfWeek = remember { state.firstVisibleWeek.days.first().date }

    LaunchedEffect(currentDate) {
        state.animateScrollToWeek(currentDate)
    }

    LaunchedEffect(state) {
        snapshotFlow { state.isScrollInProgress }.distinctUntilChanged().collectLatest {
            val firstVisibleDateOfTheWeek = state.firstVisibleWeek.days.first().date

            if (firstDateOfWeek != firstVisibleDateOfTheWeek) {
                // Week changed, make sure to refresh data.
                firstDateOfWeek = firstVisibleDateOfTheWeek
                onCalendarScroll(firstVisibleDateOfTheWeek, state.firstVisibleWeek.days.last().date)

                // Because Sunday is the first day, we need to % number of days.
                // Otherwise it doesn't work correctly if the user selected a Sunday.
                val dayOfWeek = (currentDate.dayOfWeek.value % NUM_DAYS_TO_SHOW)
                // Scroll to the same day in the previous/next week.
                onDateSelected(state.firstVisibleWeek.days[dayOfWeek].date)
            }
        }
    }

    WeekCalendar(
        state = state,
        dayContent = { day ->
            // In order to trigger re-composition of the DayCard, we need to use [derivedStateOf].
            val streakStateForDay by remember(habitDayList) {
                derivedStateOf { habitDayList?.find { it.date == day.date }?.state }
            }
            DayCard(
                date = day.date,
                selectedDate = currentDate,
                dayPosition = null,
                streakState = streakStateForDay,
                onDayClick = { clickedDate ->
                    onDateSelected(clickedDate)
                }
            )
        }
    )
@kizitonwose
Copy link
Owner

Hi, sorry I do not fully understand. In the video, I see both forward and backward scrolls. Can you explain further?

Some pointers:

val state = rememberWeekCalendarState(
    startDate = startDate,
    endDate = endDate,
    firstVisibleWeekDate = currentDate, // Do you really need to update this every time it changes? 
    firstDayOfWeek = firstDayOfWeek
)

// Why are you using the `currentDate` here?
 val dayOfWeek = (currentDate.dayOfWeek.value % NUM_DAYS_TO_SHOW)

It is difficult to tell what is incorrect since the code is incomplete. You can check if this issue is happening in the sample project or try to reproduce it there, then I can easily have a look.

@akrulec
Copy link
Author

akrulec commented Jul 25, 2024

At second 6 the backwards scroll appears, and if you look at a date, it changes from July 10th to July 3rd (at second 7), but the calendar doesn't scroll, it just skips and the icons change. Then more scrolling was supposed to happen but i just skips to June 26th, June 19th, June 12th... Thank you

@kizitonwose
Copy link
Owner

I am able to scroll fine in the sample project, so it is hard to tell what is intercepting the scroll behaviour in your app. Could it be the drawer?

scroll.mp4

@akrulec
Copy link
Author

akrulec commented Jul 25, 2024

How would I figure that out? I am still using the old Android Views for the app, and this is just a ComposeView that is part of the today screen's fragment.

@kizitonwose
Copy link
Owner

It's hard to tell since I do not have access to the project. I'd suggest isolating the calendar composable, then create and run a preview of the composable to see the issue happens there.

@akrulec
Copy link
Author

akrulec commented Jul 30, 2024

I get an error when trying to run it on device (composable not found -- probably because I have modules, and maybe because my app is still mostly set up for views). However, the interactive mode, seems to work fine. If this is a symptom of the Composable being inflated inside of a good ol' fragment, and the navigation drawer messing things up, what are my options for workarounds? It's going to be a while before everything is rewritten in compose.

@akrulec
Copy link
Author

akrulec commented Aug 6, 2024

I did find this in the main activity, but removing it didn't help the symptom:
binding.drawerLayout.setDrawerLockMode(DrawerLayout.LOCK_MODE_LOCKED_CLOSED)

@kizitonwose
Copy link
Owner

Not sure how else I can help here, unfortunately.

@kizitonwose
Copy link
Owner

kizitonwose commented Aug 10, 2024

Could you maybe try removing all calls to scrollToWeek or animateScrollToWeek in your code and initialise the calendar with static dates like below to see if the scroll issue is from your code?

    val currentDate = remember { LocalDate.now() }
    val currentMonth = remember { YearMonth.now() }
    val startDate = remember { currentMonth.minusMonths(100).atStartOfMonth() }
    val endDate = remember { currentMonth.plusMonths(100).atEndOfMonth() }
    val firstDayOfWeek = remember { firstDayOfWeekFromLocale() } 

    val state = rememberWeekCalendarState(
        startDate = startDate,
        endDate = endDate,
        firstVisibleWeekDate = currentDate,
        firstDayOfWeek = firstDayOfWeek
    )

@akrulec
Copy link
Author

akrulec commented Aug 19, 2024

I think we have hit the nail on the head. Do you have a recommended flow for the following:

  • There is a screen that has a week calendar on top, and a day's details on the bottom. When the user selects a date on the top, it updates the day's details on the bottom for the selected day.
  • The following also needs to be true:
    -- User can navigate to full screen calendar, and when they select a different date there, the week calendar needs to circle a different date, and the day's details need to change. This includes scrolling (snapping) to the correct week, and updating day's details below.
    -- When the user scrolls to a week prior, or after, we select the same day of the week in that new week (eg. Wed this week was selected, so Wed should be selected on the scroll).
    -- The information should live in ViewModel, and the UI just subscribes to the updates.

Thank you

@akrulec
Copy link
Author

akrulec commented Aug 20, 2024

I have realized that I'm missing the part where we only take into account the ! scrolling actions like here:
.filter { scrolling -> !scrolling }

That mostly solves the problem of the odd 'snap behavior' when scrolling into the past. There is still times when I try to scroll more like a fling behavior, and then it still behaves rather oddly.

@akrulec
Copy link
Author

akrulec commented Aug 20, 2024

After digging in a little more, this is what I came up with. I admit, the previous solution was mostly just a direct translation from view's based view.

@Composable
fun WeekCalendarCard(
    currentDate: LocalDate,
    onDateSelected: (LocalDate) -> Unit,
    onCalendarScroll: (LocalDate, LocalDate) -> Unit,
) {
    val currentMonth = remember { YearMonth.of(currentDate.year, currentDate.month) }
    val startDate = remember { currentMonth.minusMonths(24).atStartOfMonth() }
    val endDate = remember { currentMonth.plusMonths(2).atEndOfMonth() }
    val firstDayOfWeek = remember { DayOfWeek.SUNDAY }

    val state = rememberWeekCalendarState(
        startDate = startDate,
        endDate = endDate,
        firstVisibleWeekDate = currentDate,
        firstDayOfWeek = firstDayOfWeek)
    val visibleWeek = rememberFirstVisibleWeekAfterScroll(state)

    LaunchedEffect(currentDate) {
        // Update the state of the calendar if the date changes (by user clicking on a different
        // date in the week, or month calendar).
        state.animateScrollToWeek(currentDate)
    }

    LaunchedEffect(visibleWeek) {
        visibleWeek.days.firstOrNull()?.date?.let { firstDateAfterScroll ->
            if (firstDateAfterScroll != currentDate) {
                // Week changed, make sure to refresh data.
                visibleWeek.days.lastOrNull()?.date?.let { lastDate ->
                    onCalendarScroll(firstDateAfterScroll, lastDate)
                }

                // Select the same day in the new week, by updating the date.
                val dayOfWeek = (currentDate.dayOfWeek.value % NUM_DAYS_TO_SHOW)
                visibleWeek.days.getOrNull(dayOfWeek)?.date?.let { newDate ->
                    onDateSelected(newDate)
                }
            }
        }
    }
    ```
    
I've also noticed that setting `firstDayOfWeek` to `firstDayOfWeekFromLocale` caused issues for international users when they were selecting a new date to show from a full screen calendar (which is still written in Views). So for now, we just have all of our calendar set to have the first day of the week on Sunday.

Thank you!

@DanielRendox
Copy link

I had the same problem with monthly calendar. It was because I passed a dynamically changing currentMonth to initialMonth property, but this property should be constant.

Repository owner deleted a comment from Joshuacobarrubia Sep 9, 2024
@akrulec
Copy link
Author

akrulec commented Sep 16, 2024

I have another interesting problem now. It looks like scrolling works great (possibly because I was doing something similar that @DanielRendox mentioned), however, the date that I'm observing in view model can be changed on a different screen. For example, I open a full screen calendar fragment, select a date, and then navigate up from that screen to end up in a week view (the app is still half written in fragments/views). So when the app returns to the screen with a week view, this code takes over:

   val currentDate by todayViewModel.currentDate.observeAsState()

   val state = rememberWeekCalendarState(
        startDate = startDate,
        endDate = endDate,
        firstVisibleWeekDate = currentDate,
        firstDayOfWeek = firstDayOfWeek)
    val visibleWeek = rememberFirstVisibleWeekAfterScroll(state)

    LaunchedEffect(visibleWeek) {
        visibleWeek.days.firstOrNull()?.date?.let { firstDateAfterScroll ->
            if (firstDateAfterScroll != currentDate) {
                // Week changed, make sure to refresh data.
                visibleWeek.days.lastOrNull()?.date?.let { lastDate ->
                    onCalendarScroll(firstDateAfterScroll, lastDate)
                }

                // Select the same day in the new week, by updating the date.
                val dayOfWeek = (currentDate.dayOfWeek.value % NUM_DAYS_TO_SHOW)
                visibleWeek.days.getOrNull(dayOfWeek)?.date?.let { newDate ->
                    onDateSelected(newDate)
                }
            }
        }

So essentially the scrolling always takes over regardless of what was selected somewhere else. Is there a good way to manage this state to know whether it has been scrolled or not?

@kizitonwose
Copy link
Owner

So essentially the scrolling always takes over regardless of what was selected somewhere else

I do not understand this, please explain further.

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

No branches or pull requests

3 participants