casvanluijtelaar / paged_vertical_calendar

A simple paginated framework for implementing calendar based interfaces.
Apache License 2.0
37 stars 32 forks source link

Unable to switch between backward scrolling enabled or disabled with setState #21

Closed daveshirman closed 2 years ago

daveshirman commented 2 years ago

I have a page that shows a toggle e.g. Calendar / Tracker

As I see it, this should work fine.

However, when switching the toggle, I am unable to switch between these two calendar setups.

Test Page:

import 'package:flutter/cupertino.dart';
import 'package:flutter/material.dart';
import 'package:intl/intl.dart';
import 'package:paged_vertical_calendar/paged_vertical_calendar.dart';
import 'package:intl/intl.dart' show DateFormat;

class PageCalendarBug extends StatefulWidget {
  const PageCalendarBug({
    Key? key,
  }) : super(key: key);

  @override
  _PageCalendarBugState createState() => _PageCalendarBugState();
}

class _PageCalendarBugState extends State<PageCalendarBug> {
  bool _allowBackScrolling = false;
  int _currentTab = 0;

  @override
  void dispose() {
    super.dispose();
  }

  @override
  void initState() {
    super.initState();
  }

  void _updateView() {
    if (mounted) {
      setState(() {});
    }
  }

  @override
  Widget build(BuildContext context) {
    return SafeArea(
      child: Scaffold(
        body: Column(
          children: [
            _buildTitleToggleButton(),
            Expanded(
              child: _buildCalendar(),
            )
          ],
        ),
      ),
    );
  }

  Widget _buildCalendar() {
    DateTime now = DateTime.now();

    if (!_allowBackScrolling) {
      return PagedVerticalCalendar(
        physics: const BouncingScrollPhysics(
            parent: AlwaysScrollableScrollPhysics()),
        minDate: now,
        initialDate: DateTime(now.year, now.month, 1),
        monthBuilder: (context, month, year) =>
            _buildMonth(context, month, year),
        dayBuilder: (context, date) => _buildDay(context, date),
      );
    } else {
      return PagedVerticalCalendar(
        physics: const BouncingScrollPhysics(
            parent: AlwaysScrollableScrollPhysics()),
        initialDate: DateTime(now.year, now.month, 1),
        monthBuilder: (context, month, year) =>
            _buildMonth(context, month, year),
        dayBuilder: (context, date) => _buildDay(context, date),
      );
    }
  }

  Widget _buildMonth(BuildContext context, int month, int year) {
    return Column(
      children: [
        // create a customized header displaying the month and year
        Container(
          padding: const EdgeInsets.symmetric(vertical: 8, horizontal: 20),
          margin: const EdgeInsets.all(20),
          child: Text(
            DateFormat("MMMM, yyyy")
                .format(DateTime(year, month)),
            style: Theme.of(context).textTheme.bodyText1!.copyWith(
                  color: Colors.black,
                  fontSize: 20,
                ),
          ),
        ),

        // add a row showing the weekdays
        Padding(
          padding: const EdgeInsets.symmetric(horizontal: 20.0),
          child: Row(
            mainAxisSize: MainAxisSize.max,
            mainAxisAlignment: MainAxisAlignment.spaceBetween,
            children: [
              weekText('Mo'),
              weekText('Tu'),
              weekText('We'),
              weekText('Th'),
              weekText('Fr'),
              weekText('Sa'),
              weekText('Su'),
            ],
          ),
        ),
      ],
    );
  }

  Widget _buildDay(BuildContext context, DateTime date) {
    return Column(
      mainAxisAlignment: MainAxisAlignment.center,
      children: [
        Text(
          DateFormat('d').format(date),
          style: const TextStyle(
            fontSize: 14,
          ),
        ),
      ],
    );
  }

  Widget weekText(String text) {
    return Padding(
      padding: const EdgeInsets.all(4.0),
      child: Text(
        text,
        style: const TextStyle(color: Colors.grey, fontSize: 10),
      ),
    );
  }

  Widget _buildTitleToggleButton() {
    return CupertinoSlidingSegmentedControl(
      thumbColor: Colors.black,
      groupValue: _currentTab,
      children: {
        0: _buildToggleItem("Calendar"),
        1: _buildToggleItem("Tracker"),
      },
      onValueChanged: (int? value) {
        _currentTab = value!;
        _allowBackScrolling = !_allowBackScrolling;
        _updateView();
      },
    );
  }

  Widget _buildToggleItem(String title) {
    return Padding(
      padding: const EdgeInsets.all(4.0),
      child: Text(
        title,
        textAlign: TextAlign.center,
        style: const TextStyle(
          color: Colors.black,
        ),
      ),
    );
  }
}
casvanluijtelaar commented 2 years ago
setState(() => _allowBackScrolling = !_allowBackScrolling);
daveshirman commented 2 years ago

Erm... No? That makes no difference.

Whether you wrap code with setState or call it after making a model change, it's the same thing.

Please re-open, this bug is there. I've updated my code above so you should be able to just drop it into a plain flutter app and run it.

casvanluijtelaar commented 2 years ago

To make sure I understand it correctly, your issue is that you are trying to switch between two widgets, but that's not working for you? I don't see how this is a package specific issue?

go ahead and replace the PagedVerticalCalendar with any other widget and see if the issue persist.

otherwise take a look at the example that uses a pageview to switch between different calendars with no issue.

daveshirman commented 2 years ago

I'm trying to re-init this calendar, using a toggle to allow or disallow back scrolling.

It is certainly a package issue as it's a very relevant use case. I imagine there is some sort of caching issue when you're creating the initial calendar.

E.g in this screenshot, calendar only allows current month forward, but Tracker allows going backwards.

Make sense?

Screenshot_20220611-083704

On Sat, 11 Jun 2022, 04:51 Cas van Luijtelaar, @.***> wrote:

To make sure I understand it correctly, your issue is that you are trying to switch between 2 widgets, but that's not working for you? I don't see how this is a package specific issue?

— Reply to this email directly, view it on GitHub https://github.com/casvanluijtelaar/paged_vertical_calendar/issues/21#issuecomment-1152884384, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADM6KXRIERTJXZCTCOCMJF3VORHQ3ANCNFSM5YKMAATA . You are receiving this because you authored the thread.Message ID: @.***>

casvanluijtelaar commented 2 years ago

there might be an issue there, but I don't see that reflected in your code. you have two seperate calendar widgets here instead of something like

PagedVerticalCalendar(
   minDate: _allowBackScrolling ? null : DateTime.now(),
);
daveshirman commented 2 years ago

Then it should be even easier to diagnose?

I did have it originally as one widget like your example you've just given, but I switched to an if branch to see if it solved the issue, which it didn't.

On Sat, 11 Jun 2022, 09:09 Cas van Luijtelaar, @.***> wrote:

there might be an issue there, but I don't see that reflected in your code. you have two seperate calendar widgets here instead of something like

PagedVerticalCalendar( minDate: _allowBackScrolling ? null : DateTime.now(), );

— Reply to this email directly, view it on GitHub https://github.com/casvanluijtelaar/paged_vertical_calendar/issues/21#issuecomment-1152925681, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADM6KXSXCW6MTX32XGQTOTTVOSFZ3ANCNFSM5YKMAATA . You are receiving this because you authored the thread.Message ID: @.***>

daveshirman commented 2 years ago

Just to let you know, I implemented a PageView as you suggested thanks.

However, this is definitely still a bug..

Thanks for the suggestion

casvanluijtelaar commented 2 years ago

Just to let you know, I implemented a PageView as you suggested thanks.

However, this is definitely still a bug..

Thanks for the suggestion

Alright thanks for the report, I'll look into it soon.

casvanluijtelaar commented 2 years ago

Could you try running your app using this branch and let me know if it resolves your issues

casvanluijtelaar commented 2 years ago

I can confirm the examples behave as expected when the minDate changes, it should resolve your issue

daveshirman commented 2 years ago

Unfortunately, that's even worse! Appreciate the help and the package of course :)

This is using the example code in the top of this ticket:

https://user-images.githubusercontent.com/14280030/173369937-8a5b37d5-27a4-464a-8eb1-5eacbe494b04.mov

casvanluijtelaar commented 2 years ago

did you try the latest change to that branch +-3 hours ago? I cannot replicate what's going on there, can you please create a minimum reproducible example?

for instance I am using the example app included in the package. where everything seems to work fine.

daveshirman commented 2 years ago

Yeah - I used the code I provided above (which is exactly what is running in that video) as I said with a dependency override like so:

dependency_overrides:
   paged_vertical_calendar:
     git:
       url: https://github.com/casvanluijtelaar/paged_vertical_calendar/
       ref: update_init_values_on_rebuild

In all honesty, if this is a time-sink, don't worry about it. It's not a showstopper.

casvanluijtelaar commented 2 years ago

I tried your scenario and indeed noticed a little issue that cause the null check error, pushed an update to that branch. It's a bit tricky to know what months to update when minDate is changed to null, for now something like this should work for you:

PagedVerticalCalendar(
  physics: const BouncingScrollPhysics(
    parent: AlwaysScrollableScrollPhysics(),
  ),
  minDate: _allowBackScrolling ? DateTime(1900) : now,
  initialDate: DateTime(now.year, now.month, 1),
  monthBuilder: _buildMonth,
  dayBuilder: _buildDay,
),

I'm not quite happy with the implementation though, I'll properly look into it next weekend or so, for now you can keep using this branch

casvanluijtelaar commented 2 years ago

this will now be available in master, I'll leave this branch open for a couple weeks so you can migrate. this fix should be available in 1.1.5 when it is released.