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

added an initial calendar date to allow for bi-directional pagination #13

Merged
merged 13 commits into from
Feb 26, 2022

Conversation

desmeit
Copy link
Contributor

@desmeit desmeit commented Jan 27, 2022

Added the functionality to use an initial date to which the calendar jumps at start. I have orientated on this principle:
EdsonBueno/infinite_scroll_pagination#56 (comment)
If no startDate is given, it takes DateTime.now(). same for initDate. If initDate is larger than endDate, it is taken endDate.
In addition I added a localization parameter and weekdays as option.

Example:

PagedVerticalCalendar(
      invisibleMonthsThreshold: 1,
      startDate: DateTime(2021, 9, 1),
      endDate: DateTime(2022, 3, 1),
      initDate: DateTime(2022, 2, 1),
      weekDays: true,
      languageCode: 'de',
      onMonthLoaded: (year, month) {
        // on month widget load
      },
      onDayPressed: (value) {
        // on day widget pressed
      },
      onPaginationCompleted: () {
        // on pagination completion
      },
    )

@desmeit desmeit changed the title Added localization, initialIndex and weekdays Added localization, initDate and weekdays Jan 27, 2022
Copy link
Owner

@casvanluijtelaar casvanluijtelaar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll test it this weekend, make sure to remove any (commented) print statements

@casvanluijtelaar
Copy link
Owner

Alright, interesting work. I like the initDate feature to allow pagination in both directions. this would be a nice addition to the package. However tests are currently failing and i'd recommend adding a few more to make sure it works.

you're trying to push several features in one commit, I wouldn't recommend that.

weekdays is feature that won't be added. this package is a minimal framework for creating your own looks. the included example already shows how to add weekdays to the top of every month.

In the same reasoning, included localisation is not needed since you can customize any date visualizations using the builder.

tldr, remove the weekdays and localization and fix the minor issues and I'll happily get this merged. Thanks for your work!

@casvanluijtelaar casvanluijtelaar changed the title Added localization, initDate and weekdays added an initial calendar date to allow for bi-directional pagination Jan 28, 2022
final String? languageCode;

/// init with this Date. If no startDate is given, it takes DateTime.now().
final DateTime? initDate;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call this initialDate and add a bit more documentation. something along the lines of the initial date that should be displayed on the calendar

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your feedback. I will do that.

Copy link
Contributor Author

@desmeit desmeit Jan 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the confusing commits. This is my first pull request. ;-) I had problems with VS code. Future commits should be better named.
I have improved the logic for the calculation of the initialDate as well. Please take a look.

@casvanluijtelaar
Copy link
Owner

thanks for still working on this! I haven't forgotten this pull request, I have been very busy. I'll find some time to integrate this one of these days.

@casvanluijtelaar casvanluijtelaar merged commit 8b6cf2a into casvanluijtelaar:master Feb 26, 2022
@casvanluijtelaar
Copy link
Owner

now available on pub..dev thanks for your contribution! 🎉

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

Successfully merging this pull request may close these issues.

3 participants