thellmund / Android-Week-View

Display highly customizable calendar views in your Android app
Apache License 2.0
188 stars 97 forks source link

goToDate() behavior inconsistent #99

Closed nicholas-gs closed 5 years ago

nicholas-gs commented 5 years ago

Describe the bug The widget was not scrolling to the date I passed it. While debugging, I removed all other functions/functionality and found that goToDate() behavior is affected by setHeaderRowTextSize().

To Reproduce Steps to reproduce the behavior:

   @Override
    public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceState) {
        super.onViewCreated(view, savedInstanceState);

        mWeekView = view.findViewById(R.id.home_fragment_weekview);
        mWeekView.setMonthChangeListener(this);
        mWeekView.setOnEventClickListener(this);
        mWeekView.setScrollListener(this);

        mWeekView.setHeaderRowTextSize(getResources().getInteger(R.integer.HEADER_14_SP));

        Calendar calendar = Calendar.getInstance();
        calendar.set(Calendar.MONTH, 10);
        calendar.set(Calendar.DAY_OF_MONTH, 5);
        mWeekView.goToDate(calendar);
    }

With the above code, the widget does not scroll to 5 Nov!

However, if I removed the line mWeekView.setHeaderRowTextSize(getResources().getInteger(R.integer.HEADER_14_SP));, the widget scrolls correctly to 5 Nov.

Interestingly, if I were to move that line below mWeekView.goToDate(calendar); like below, the widget scrolls correctly as well!

    @Override
    public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceState) {
        super.onViewCreated(view, savedInstanceState);

        mWeekView = view.findViewById(R.id.home_fragment_weekview);
        mWeekView.setMonthChangeListener(this);
        mWeekView.setOnEventClickListener(this);
        mWeekView.setScrollListener(this);

        Calendar calendar = Calendar.getInstance();
        calendar.set(Calendar.MONTH, 10);
        calendar.set(Calendar.DAY_OF_MONTH, 5);
        mWeekView.goToDate(calendar);

        mWeekView.setHeaderRowTextSize(getResources().getInteger(R.integer.HEADER_14_SP));
    }

Additional context

nicholas-gs commented 5 years ago

goToDate() also affected by setEventHeight()

The function setEventHeight() also affects goToDate() the same way as setHeaderRowTextSize() does as described above. Removing the function call or calling it after setHeaderRowTextSize() removes the bug.

thellmund commented 5 years ago

Hi @wRorsjakz! The calls to goToDate() and setHeaderRowTextSize() both result in invalidate() being called (see here and here), which apparently causes this issue when called in rapid succession.

I haven’t yet found a good solution to address this. For now, my advice would be to either move setHeaderRowTextSize() below goToDate() (as you’ve already done) or to set the header row text size in XML.

nicholas-gs commented 5 years ago

Thank you for your reply.

I managed to address the problem by ordering the function calls like you said. It was just that I had to programmatically change the header row text size based on the number of visible days the user selected for the widget, else the headers would overlap. Just wanted to point out the bug.

thellmund commented 5 years ago

I’ll keep this issue open and will let you know once I’ve come up with a fix. If you want to contribute yourself, feel free to do so.

thellmund commented 5 years ago

I was wrong: The issue was not related to the invalidation, but to the fact that WeekView hadn't been laid out yet when you called goToDate(). I fixed this issue in 43a3a6eb767eed8f4469f2d3af7faf20f9b1a1c2 and will release it with version 4.0.0 around mid-August.

I will close this issue now. Let me know if there’s anything else that’s not working as expected.