Feature/weekend days #576
Feature/weekend days #576
Conversation
- add holiday Days feature - update gradle
-disabled weekend days -update grade
-disabled weekend days - update gradle
|
This looks like a good feature to have. |
|
Thanks , I wait for review my commit
…On Tue, Apr 9, 2019, 6:33 PM wdullaer ***@***.***> wrote:
This looks like a good feature to have.
I will try to do a detailed review later this week.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#576 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/Au4hey1_PLtdYMjDkIt9Ud8q-wKetiBYks5vfMDXgaJpZM4cWEWn>
.
|
| @@ -6,7 +6,7 @@ buildscript { | |||
| jcenter() | |||
| } | |||
| dependencies { | |||
| classpath 'com.android.tools.build:gradle:3.2.1' | |||
| classpath 'com.android.tools.build:gradle:3.3.2' | |||
There was a problem hiding this comment.
Please don't update gradle in a feature PR, unless it's required for the functionality you are building.
This avoids merge conflicts if gradle gets upgraded in master before your PR lands
| * | ||
| * @param weekendDays an Array of Calendar Objects containing the disabled dates | ||
| */ | ||
| public void setWeekendDays(@NonNull List<Integer> weekendDays) { |
There was a problem hiding this comment.
I don't like the naming here. There's nothing about a weekend day which implies that it's disabled. Some people might have the exact opposite requirement.
Maybe setDisabledWeekDays?
| * | ||
| * @param weekendDays an Array of Calendar Objects containing the disabled dates | ||
| */ | ||
| public void setWeekendDays(@NonNull List<Integer> weekendDays) { |
There was a problem hiding this comment.
Maybe we can use this in the signature? https://docs.oracle.com/javase/8/docs/api/java/time/DayOfWeek.html (it has utility methods to convert to a Calendar int later on).
That might be a more intuitive signature than int.
| @@ -143,7 +164,8 @@ public int getMaxYear() { | |||
| } | |||
|
|
|||
| @Override | |||
| public @NonNull Calendar getStartDate() { | |||
| public @NonNull | |||
| Calendar getStartDate() { | |||
There was a problem hiding this comment.
You need to make sure the start date is not on a disabled weekday
| @@ -155,7 +177,8 @@ public int getMaxYear() { | |||
| } | |||
|
|
|||
| @Override | |||
| public @NonNull Calendar getEndDate() { | |||
| public @NonNull | |||
| Calendar getEndDate() { | |||
There was a problem hiding this comment.
You need to make sure the end date is not on a disabled weekday
| @@ -203,7 +232,8 @@ private boolean isAfterMax(@NonNull Calendar calendar) { | |||
| } | |||
|
|
|||
| @Override | |||
| public @NonNull Calendar setToNearestDate(@NonNull Calendar calendar) { | |||
| public @NonNull | |||
| Calendar setToNearestDate(@NonNull Calendar calendar) { | |||
There was a problem hiding this comment.
setToNearest will need to take the disabled weekdays into account (you don't want to return a date that is disabled)
| <LinearLayout | ||
| android:orientation="vertical" android:layout_width="match_parent" |
| @@ -39,8 +42,10 @@ | |||
| private Calendar mMaxDate; | |||
| private TreeSet<Calendar> selectableDays = new TreeSet<>(); | |||
| private HashSet<Calendar> disabledDays = new HashSet<>(); | |||
| private List<Integer> weekendDays = new ArrayList<>(); | |||
There was a problem hiding this comment.
Maybe a set makes more sense than a list?
-disabled weekend days