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

VisibleDateRange.days(7,...) behavior #93

Closed
ThexXTURBOXx opened this issue Aug 3, 2021 · 4 comments
Closed

VisibleDateRange.days(7,...) behavior #93

ThexXTURBOXx opened this issue Aug 3, 2021 · 4 comments
Labels
T: Fix Type: :bug: Bug Fixes

Comments

@ThexXTURBOXx
Copy link
Contributor

ThexXTURBOXx commented Aug 3, 2021

Describe the bug

If I use a VisibleDateRange.days or VisibleDateRange.weeks and set their maxDate, the calculations seem to be slightly off what I expect. For example, if I use a VisibleDateRange.days(7,...) and set its minDate to today and maxDate to be 9 days in the future, the shown maxDate ends up at 15 days in the future. If you want to see the code, this is the line I am talking about.
The MultiDateTimetableContent seems to be working correctly, but the DateHeader behaves somehow weird.
As shown below, in the videos section, for some reason, the day range gets added to the maxDate, which is counterintuitive in this case.

Taken from #91. Here now reported as a separate bug.

Videos

In the video I show the following behavior:

  • Setting maxDate to minDate + 9.days with VisibleDateRange.days(7) yields 16 shown dates (9 + 7 = 16)
  • Setting maxDate to minDate + 10.days with VisibleDateRange.days(7) yields 17 shown dates (10 + 7 = 17)
  • Setting maxDate to minDate + 1.days with VisibleDateRange.days(7) yields 8 shown dates (1 + 7 = 8)

https://www.youtube.com/watch?v=Lk7zecYq4L8

Environment:

  • Device: Emulator Pixel 4
  • OS version: Android 12 (SDK 31 Preview)
  • Package version: 1.0.0-alpha3
@ThexXTURBOXx ThexXTURBOXx added the T: Fix Type: :bug: Bug Fixes label Aug 3, 2021
@paolovalerdi
Copy link
Contributor

paolovalerdi commented Aug 4, 2021

I actually came across this 'issue' before.
I'm pretty sure this is intended behavior because of the swipeRange property.

So the workaround is to actually set maxDate the same value as minDate, that'll give you the behavior you're expecting.

Edit:
The reason you see more days than expected it's because of the swipeRange prop, as you need to have at least one page to 'swipe' when setting visibleDayCount to any other value than 1, you'll end up having maxDate + visibleDayCount days more because that fulfills the swipeRange value, that's why settings maxDate = minDate works because you'll have minDate + visibleDayCount which is what you expect.

@ThexXTURBOXx
Copy link
Contributor Author

I'm not sure if I understand correctly. I want the DateHeader to show dates between today and today + 15.days. The dates should be aligned in weeks. So, today will be the first day in the DateHeader (thus, the minDate) and today + 15.days should be the last one (thus, the maxDate).
The DateHeader should show one week at a time, but with a swipeRange of 1 day. Thus, I set the visibleDayCount to 7 and let the swipeRange be the default for VisibleDateRange.days(7,...), which is 1.
This should be possible to show correctly, even without needing to fill unwanted days.

If I wanted to set the maxDate to e.g. today + 2.days, I would expect one of two things:

  • There are only 3 days shown in the header: today, tomorrow, the day after tomorrow, or
  • There are still 7 days shown in the header: the same as above and some dates to fill up to 7 days.

There is, however, a problem with the second solution: Which dates should you choose? Should you choose 2 days before the minDays and two dates after the maxDate or just 4 days either before the minDate or after the maxDays or something else?
So, the second solution is ambiguous and should most likely be avoided. That's why for example material-calendarview chose the first solution by default with possible customization to choose the second one.

Now, to my issue again: I wanted to show everything between today and today + 15.days while showing 7 days at a time, but with a swipeRange of 1. Since there are 16 days to show in total, the DateHeader will always be able to show 7 days at once and wouldn't even need one of the two solutions from above (I can even "force" this behavior with the workaround I stated in the first post: Setting maxDate to today + 9.days).
This is why it is in most cases counter-intuitive to set the actually shown maximum date to something that is completely different from the maxDate being passed to the VisibleDateRange.
And, as demonstrated with the two cases above, it is even counter-intuitive to tamper with the maxDate without the user's knowledge in any case.

Because of these reasons, I still think that this is a bug. Please correct me if I am wrong.

@JonasWanke
Copy link
Owner

With swipeRange being one, the behavior was indeed incorrect but is now fixed in v1.0.0-alpha.4.

Regarding the case of visibleDayCount: 7, minDate: today, maxDate: today + 2days described by @ThexXTURBOXx in the comment above: minDate's and maxDate's only purpose is to limit the swipeable range. Therefore, showing only three days would not fit the current intention of the parameters as the visibleDayCount is clearly defined. (But you could easily get this behavior by setting visibleDayCount to math.min(7, maxDate.difference(minDate).inDays) instead of just 7.)

Timetable, therefore, uses the second approach. The visible range is hereby aligned as far left as possible in the visible area, as far as swipeRange + alignmentDate allow (e.g., with the default swipeRange of one, minDate is the left-most date). The extra dates shown are then to the right of the selected range.

@ThexXTURBOXx
Copy link
Contributor Author

Thank you very much! I can confirm that both bugs are fixed in the new update :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: Fix Type: :bug: Bug Fixes
Projects
None yet
Development

No branches or pull requests

3 participants