wdullaer / MaterialDateTimePicker

Pick a date or time on Android in style
Apache License 2.0
4.67k stars 948 forks source link

Clear disabledDays field when calling setDisabledDays() #674

Open premacck opened 4 years ago

premacck commented 4 years ago

I have a requirement where I have a list of entities, and every entity has their own set of disabledDays So whenever I select an entity, I have to set it's disabledDays to the DatePickerDialog.

I was trying to maintain a single instance of DatePickerDialog so I don't have to re-create it every time I have to show it. I was hoping that if I just call the setDisabledDays(...) method and set the disabled dates, the new dates passed would overwrite the previous values and be disabled.

So when calling datePickerDialog.setDisabledDays(disabledDays.distinct()), it is working as expected for the first selection, but for the next selection, when it is called again on the same DatePickerDialog instance, the new dates are added to the mDefaultLimiter.disabledDays set.

Currently the only way to avoid this issue is to create a new DatePickerDialog instance everytime we need it, but an other alternative (using the same instance) can be:

Inside the DefaultDateRangeLimiter class, we have:

    void setDisabledDays(@NonNull Calendar[] days) {
        for (Calendar disabledDay : days) {
            this.disabledDays.add(Utils.trimToMidnight((Calendar) disabledDay.clone()));
        }
    }

For me, the method name is a bit confusing, because it "adds" the disabled dates to the already existing list, instead of setting it.

My suggestion is to add a new method to replace the disabled dates like this:

    void replaceDisabledDays(@NonNull Calendar[] days) {
        this.disabledDays.clear();
        addDisabledDays(days);
    }

I think a method like addDisabledDates(...) can be more right for the users to know that the dates are being added, not replaced, So we can move the logic of setDisabledDates() method to addDisabledDates(), call the latter from the former, and deprecate the former method.

    void addDisabledDays(@NonNull Calendar[] days) {
        for (Calendar disabledDay : days) {
            this.disabledDays.add(Utils.trimToMidnight((Calendar) disabledDay.clone()));
        }
    }

    /** @deprecated Use {@link #addDisabledDays(Calendar[])} instead */
    @Deprecated
    void setDisabledDays(@NonNull Calendar[] days) {
        addDisabledDates(days)
    }
premacck commented 4 years ago

@wdullaer Created a pull request for the same: #675