fix: only decorate the content of the modal

[benthillerkus]

Description

This narrows the part of the tree on to which the user-provided decorator function is applied to to just the modal, instead of also wrapping the barrier.

Related Issues

• The child provided to decorator functions is larger than it should be #201

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process.

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• My PR includes tests for all changed/updated/fixed behaviors.
• All existing and new tests are passing.
• I updated/added relevant documentation (doc comments with ///).
• The analyzer (melos run analyze) does not report any problems on my PR.
• The package compiles with the minimum Flutter version stated in the pubspec.yaml

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

• Yes, this is a breaking change.
• No, this is not a breaking change.

[github-actions]

Visit the preview URL for this PR (updated for commit 15d867b):

• https://coffeemakerexample--pr203-only-decorate-conten-yri7tx6o.web.app
• https://playgroundwoltexample--pr203-only-decorate-conten-mjz6e37x.web.app
• https://playgroundnavigator2woltexample--pr203-only-decorate-c-w77xflwc.web.app

_{(expires Thu, 16 May 2024 23:15:42 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: 906758393beb0353b979d020649d6a1efc40fb5b}

[ulusoyca]

@benthillerkus This is a good catch! Thanks. The next release will be very soon. In fact the documentation states that the decorator decorates the content.

[vishna]

@ulusoyca I've updated from 0.5.x to 0.7.x . The context that comes with pageListBuilder no longer contains the bloc added via decorator ( meaning calling context.read will throw ). I now have to wrap everything with Builder which is a less convenient thing to do. Looking at the changelog this PR would be the one that changed old behavior. Sadly I don't have time to investigate more/confirm this.

[ulusoyca]

@vishna This is strange because we have the coffee maker demo app that utilizes the provider added by the decorator.

The demo app works fine. https://github.com/woltapp/wolt_modal_sheet/blob/main/coffee_maker/lib/home/online/store_online_content.dart#L104

Please let me know if there is a bug. We can quickly make a hotfix.

[vishna]

@ulusoyca your demo works correct as it is, however if you would modify:
https://github.com/woltapp/wolt_modal_sheet/blob/main/coffee_maker/lib/home/online/modal_pages/grind/grind_or_reject_modal_page.dart#L11 to take BuildContext as an argument (so in other words reuse context provided by pageListBuilder):

static WoltModalSheetPage build({required BuildContext context, required String coffeeOrderId})

and then replace e.g.:

Builder(builder: (context) {
  final model = context.read<StoreOnlineViewModel>();
  return WoltElevatedButton(
    onPressed: () {
      model.onCoffeeOrderStatusChange(
          coffeeOrderId, CoffeeMakerStep.addWater);
      Navigator.pop(context);
    },
    child: const Text('Start grinding'),
  );
}),

with

WoltElevatedButton(
  onPressed: () {
    final model = context.read<StoreOnlineViewModel>();
    model.onCoffeeOrderStatusChange(
        coffeeOrderId, CoffeeMakerStep.addWater);
    Navigator.pop(context);
  },
  child: const Text('Start grinding'),
);

this would fail probably in 0.7.0 but would pass in 0.5.0.

Maybe you don't need context in the pageListBuilder anymore 🤔 (what is it good for anyway?) ...or maybe this should be added as a note to changelog 🤔

[vishna]

Anyway updated my code not to use context from pageListBuilder, had to wrap a bunch of things with Builder and it works again.

[ulusoyca]

Thanks for debugging! I will make sure this is addressed correctly.

[ulusoyca]

Hi @vishna ! Thank you very much for detailed debugging. I addressed this issue in this PR: #267 What do you think? Can you review? To test this PR, I updated the coffee maker demo app.