playground examples not working with one click

[AcarFurkan]

Bug report

I take this error both in playground and playground_navigator2.After opening wolt_modal_sheet, I cannot select an element in the list with a single click.In the playground example, I cannot select it at all. In the playground_navigator2 example, I can select it with double click. I think the reason for this in playground is that pageListBuilder is called with every change, so it is rebuilt each time.

wolt_modal_sheet/playground/lib/home/pages/multi_page_path_name.dart

Line 32 in 3da966e

WoltModalSheetPageListBuilder pageListBuilder({

I was going to look into how to fix this, but I didn't understand whether it was an undesirable behavior or whether it was done this way on purpose.

Steps to reproduce

Steps to reproduce the behavior:

Playground Example	Playground Nav 2 Example

Expected behavior

I think we should be able to select it with a click

[ulusoyca]

Thanks for the report. It looks like 0.4.0 has issues. 0.3.0 should still be working. We are on it.

[ulusoyca]

@AcarFurkan We found out the root cause, and we will release the hotfix tomorrow.

[AcarFurkan]

@ulusoyca First of all, thank you for your attention so quickly :) I also found the reason why the error did not occur in the past but now. Actually, the _addPage method is called again in the else section and it calls _createIncomingWidgets again. I wanted to ask because I was curious. Why did we need to renew the _incomingPageWidgets in case the index did not change? What kind of case did you have to do this for? I would be very happy if you would like to answer. Thank you, it is a very nice and detailed repo :)
https://github.com/woltapp/wolt_modal_sheet/blob/3da966e3ba1e94f53e45c7373c84ff74e30ff93c/lib/src/content/wolt_modal_sheet_animated_switcher.dart#L194C1-L205C1

[ulusoyca]

Thanks for checking the issue. Yes this is because of the lines you pointed.

WoltModalSheet has been an internal component at Wolt for around 3 years. We open sourced this component less than a year ago. The modal sheet pages were not designed to be widgets, but rather configurations to build a bigger widget like a Voltran. We are at a point that it takes significant time resources to make WoltModalSheetPage a widget rather than a data holder class.

Before 0.4.0, the state management through the decorator widget worked well as long as the same widgets are used for different states.

When the state changes, if the page content completely changes, these changes would not be reflected because the current page widgets are not re-created.

For example, this would not work (TaskItemHistoryDetailViewModel is injected through decorator).

      pageListBuilder: (context) {
        final viewModel = context.watch<TaskItemHistoryDetailViewModel>();
        final state = viewModel.state;

        return [
          SliverWoltModalSheetPage(
            mainContentSlivers: state.viewState.when(
              loading: () => const [
                SliverToBoxAdapter(
                  child: Center(
                    child: Padding(
                      padding: EdgeInsets.all(sp16),
                      child: LoadingIndicator(size: _loadingIndicatorSize),
                    ),
                  ),
                ),
              ],
              idle: () => TaskItemDetailModalContent.createSlivers(
                aggregatedItem: aggregatedTaskItemHistory,
                options: state.taskItemOptionsHistory.options,
              ),
              error: () => [
                SliverToBoxAdapter(
                  child: FullScreenErrorWidget(
                    error: state.error ?? ParsedError(error: Object()),
                    action: () => viewModel.onInit(
                      aggregatedTaskItemHistory,
                      selectedPastOrdersFilterSet,
                    ),
                  ),
                ),
              ],
            ),
          ),
        ];
      },

My change attempted to re-create current pages everytime the page state changed, but this ended up losing the states of the current widgets. We should have done better testing. Anyway, a hotfix is coming.

The best way to update the widgets according to state:

1. Use decorator to mount the ViewModel or ChangeNotifierProvider or BlocProvider or. your fav. state management state holder widget.
2. Expose value listenables from the ViewModel or use ViewModel directly if it is already a ValueListenable.
3. Wrap the page components with ValueListenableBuilder or ListenableBuilder.

This workaround would work.

    return WoltModalSheet.show(
      context: context,
      pageListBuilder: (context) {
        final viewModel = context.read<TaskItemHistoryDetailViewModel>();

        return [
          WoltModalSheetPage(
            child: ListenableBuilder(
              listenable: viewModel,
              builder: (context, _) {
                final state = viewModel.state;
                final viewState = state.viewState;

                return viewState.when(
                  loading: () => const Center(
                    child: Padding(
                      padding: EdgeInsets.all(sp16),
                      child: LoadingIndicator(size: _loadingIndicatorSize),
                    ),
                  ),
                  idle: () => TaskItemDetailModalContent(
                    aggregatedItem: aggregatedTaskItemHistory,
                    options: state.taskItemOptionsHistory.options,
                  ),
                  error: () => FullScreenErrorWidget(
                    error: state.error ?? ParsedError(error: Object()),
                    action: () => viewModel.onInit(
                      aggregatedTaskItemHistory,
                      selectedPastOrdersFilterSet,
                    ),
                  ),
                );
              },
            ),
          ),
        ];
      },
      decorator: (widget) => ViewModelProvider<TaskItemHistoryDetailViewModel>(
        builder: (_, model, __) => widget,
        onViewModelProvided: (viewModel) => viewModel.onInit(
          aggregatedTaskItemHistory,
          selectedPastOrdersFilterSet,
        ),
      ),
    )

The code above is for this screen:

past_order_fix.mp4

[AcarFurkan]

I understand it better now, thank you very much :) Sometimes it can be very difficult to transfer projects to open source. While you can solve everything internally with your methods, it can be very difficult to open it up and make it work for everyone. But a more modular code will certainly emerge at the end of the day

[ulusoyca]

@AcarFurkan If we had integration tests in place, we would have been in safer spot. Working on it. Closing this as we reverted the root cause.