minikin / popover

Popover for Flutter. A popover is a transient view that appears above other content onscreen when you tap a control or in an area.
https://pub.dev/packages/popover
MIT License
166 stars 54 forks source link

Popover not visible on screen if called inside of SliverAppBar action #37

Closed JonasHiltl closed 2 years ago

JonasHiltl commented 3 years ago

Describe the bug I'm calling the showPopover function from an action of the SliverAppBar but the Popover is not shown on the screen. I guess that it is positioned outside of the screen.

Code:

class ManageDocuments extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    final sessionState = context.watch<Verified>();
    return CustomScrollView(
      physics: const BouncingScrollPhysics(),
      slivers: <Widget>[
        SliverAppBar(
          // if floating is true the appbar becomes instantly visible if scrolled towards top
          // if it's false the appbar is only visible if completly scrolled back to top
          floating: true,
          expandedHeight: 60.0,
          backgroundColor: Theme.of(context).scaffoldBackgroundColor,
          title: Text(
            "Manage Documents",
            style: TextStyle(color: Theme.of(context).colorScheme.onBackground),
          ),
          centerTitle: true,
          actions: [
            IconButton(
              icon: Icon(
                Icons.add,
                color: Theme.of(context).colorScheme.onBackground,
              ),
              onPressed: () => showPopover(
                context: context,
                bodyBuilder: (context) => Column(
                  children: [
                    Text(L.of(context).patientQuestionnaire),
                    Text(L.of(context).patientQuestionnaire),
                    Text(L.of(context).patientQuestionnaire),
                  ],
                ),
                direction: PopoverDirection.bottom,
              ),
            ),
          ],
        ),
      ],
    );
  }
}

Expected behavior I expected the Popover to be positioned below the IconButton in the appBar, with the direction of PopoverDirection.bottom

Screenshots I already clicked on the action IconButton and the background get's dimmed but the popover is out of screen. Unbenannt

Smartphone (please complete the following information):

minikin commented 3 years ago

@JonasHiltl Thank you for your report. I'll take a look in the next couple of days.

minikin commented 3 years ago

@JonasHiltl I cannot reproduce it. So I created a project to play with a popover. Would you mind creating a reproducible example there?

JonasHiltl commented 3 years ago

I just ran your test project and could reproduce the issue. I'm running it on a Pixel C emulator and the popover doesn't show but the screen darkens, as seen in the image above. I guess that the issue either is caused by the Pixel C device or the CustomScrollView which might position the popover outside of the screen.

Liliana99 commented 3 years ago

Same issue, In my case I have a floating Navigation Bar, the body of the screen is a SingleChildScrollView, the behaviour at the moment to call popover is not visible, at the background is appears somethings like these screen as @JonasHiltl describe.

minikin commented 3 years ago

@Liliana99 @JonasHiltl Thanks, I was able to reproduce it. I'll try to fix it asap. If you have ideas on how to fix it, you're very welcome πŸ˜‰

pythonhubdev commented 3 years ago

Facing the same issue. I think this needs to be fixed asap.

pythonhubdev commented 3 years ago

It's not working in a SingleChildScrollView as well I tried it out.

What may be causing this issue any idea? So that I could help as well

minikin commented 3 years ago

@pythonhubpy @JonasHiltl @Liliana99 I did some investigation on a weekend. I discovered that widgets are actually present in the widgets tree. But I didn't have enough time to investigate why they are not displayed on a screen. Any thoughts?

pythonhubdev commented 3 years ago

Trying to figure it out. Should we use some Sliver widgets? Would it may be a cause?

minikin commented 3 years ago

@pythonhubpy I'm not sure. We (or I) implemented something in the wrong way, because this thing works.

pythonhubdev commented 3 years ago

@minikin I tried out with RenderSliverToBoxAdapter, RenderSliverSingleBoxAdapter and RenderSliverMultiBoxAdapter. Nothing worked and I am getting some error even.

A RenderStack expected a child of type RenderBox but received a child of type PopoverPositionRenderObject.

I think the problem would be while rendering the widget in a ScrollView because the ScrollView position keeps on changing while we scroll so I think this could affect the rendering.

pythonhubdev commented 3 years ago

I have a question can't we render this with an Overlay widget provided by flutter. We actually just need to calculate the position for the OverLayEntry I don't know whether it's a good idea just asking for suggestion @minikin.

minikin commented 3 years ago

@pythonhubpy Thanks for trying things, really appreciate it!

Yes, we can use Overlay but it has other implications in complex systems (think about something like Netflix)

I think that we need to start here and dig deep if needed.

We have this one #21 for a long time. Maybe that's the moment to finally do it πŸ€“ The idea is to make it work like CupertinoActionSheet works.

Plus, we need to keep in mind the backward capability of public API.

pythonhubdev commented 3 years ago

Will work on it.

pythonhubdev commented 3 years ago

Tried making different changes in PopoverItem.dart but couldn't resolve the issue. Any ideas @minikin

minikin commented 3 years ago

@pythonhubpy I'll back from vacation next week and will have a look in greater detail into this issue.

pythonhubdev commented 3 years ago

Sure

IgorDevJS commented 3 years ago

hi guys, i'm br so sorry my english haha

I was getting the same error inside SliverPersistentHeader when calling the showPopover function from inside this widget but I managed to solve it by creating a new widget just for the button that will call the popover, thus passing the context of the button Widget, and now it works

Here's an example of the workaround I made: https://github.com/IgorDevJS/popover_playground/commit/2baeb14e11b5e62b060afdfff3ea711f6537ca64

minikin commented 3 years ago

@IgorDevJS thank you very much for sharing πŸ™

It looks like it's only partially fix the issue for me:

Screenshot 2021-07-26 at 9 49 15 AM
IgorDevJS commented 3 years ago

You are welcome!

Yes true.

I quickly googled and found this, I don't know if it can improve https://github.com/flutter/flutter/issues/31059#issuecomment-500961641

When I have some time here I can try to apply and run some tests.

minikin commented 3 years ago

@IgorDevJS I don't think it can help us here. Moreover, as you can see on a screenshot, a layout of a popover is broken.

IgorDevJS commented 3 years ago

@minikin Hmm OK

But the broken layout problem I don't think is related to the popover problem in SliverAppBar

It seems to be because the button is too far in the corner and because it is too small the arrow ends up outside the popover

In my project I'm developing, the button has a larger width and the popover layout is ok

pythonhubdev commented 3 years ago

Any updates on this @minikin?

minikin commented 3 years ago

@pythonhubpy Unfortunately, I don't have time now to work on this issue. However, if someone finds a solution, I'd be happy to review PR and test a solution.

pythonhubdev commented 3 years ago

Tried out different ways I couldn't find out what is the exact issue.

felipecs81 commented 3 years ago

You need to pass the correct context when displaying the popOver menu. In your code you are passing the context of the ManagerDocument Widget, so the menu is showing off screen. You have two options in this case, either putting it inside another builder or creating a specific widget for it to pass its context.

DenisGL commented 3 years ago

@felipecs81 , I've tried creating a specifing widget... and it works! Thanks a lot

minikin commented 3 years ago

@felipecs81 @DenisGL Should we add this trick to docs πŸ€”

DenisGL commented 3 years ago

@minikin , for sure: without this "trick" I was stuck, and could not use this package at all (all my popovers are triggered from buttons in SliverAppBar widgets)

minikin commented 3 years ago

@DenisGL Would you like to share a quick demo (code) of what actually users need to do to make it work?

DenisGL commented 3 years ago

@minikin , yes. I use "Cupertino" widgets, but I suppose this can be adapted to Material Design. Before (not working code, i.e. the screen is dimmed and no popover is displayed):

CupertinoSliverNavigationBar(
    largeTitle: 'Title',
    trailing:  CupertinoButton(
        padding: EdgeInsets.zero,
        child: const Icon(CupertinoIcons.textformat_size, size: 28),
        onPressed: () => showPopover(context: context, ...)
    ),
)

After:

CupertinoSliverNavigationBar(
    largeTitle: 'Title',
    trailing:  PopButton(),
)
...
class PopButton extends StatelessWidget {
  const PopButton({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return CupertinoButton(
        padding: EdgeInsets.zero,
        child: const Icon(CupertinoIcons.list_bullet, size: 28),
        onPressed: () {
          showPopover(context: context, ...)
        }
    );
  }
}
arsize commented 3 years ago

@minikin , yes. I use "Cupertino" widgets, but I suppose this can be adapted to Material Design. Before (not working code, i.e. the screen is dimmed and no popover is displayed):

CupertinoSliverNavigationBar(
    largeTitle: 'Title',
    trailing:  CupertinoButton(
        padding: EdgeInsets.zero,
        child: const Icon(CupertinoIcons.textformat_size, size: 28),
        onPressed: () => showPopover(context: context, ...)
    ),
)

After:

CupertinoSliverNavigationBar(
    largeTitle: 'Title',
    trailing:  PopButton(),
)
...
class PopButton extends StatelessWidget {
  const PopButton({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return CupertinoButton(
        padding: EdgeInsets.zero,
        child: const Icon(CupertinoIcons.list_bullet, size: 28),
        onPressed: () {
          showPopover(context: context, ...)
        }
    );
  }
}

but,the background mask covers pop-up window...

arsize commented 3 years ago

@minikin , yes. I use "Cupertino" widgets, but I suppose this can be adapted to Material Design. Before (not working code, i.e. the screen is dimmed and no popover is displayed):

CupertinoSliverNavigationBar(
    largeTitle: 'Title',
    trailing:  CupertinoButton(
        padding: EdgeInsets.zero,
        child: const Icon(CupertinoIcons.textformat_size, size: 28),
        onPressed: () => showPopover(context: context, ...)
    ),
)

After:

CupertinoSliverNavigationBar(
    largeTitle: 'Title',
    trailing:  PopButton(),
)
...
class PopButton extends StatelessWidget {
  const PopButton({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return CupertinoButton(
        padding: EdgeInsets.zero,
        child: const Icon(CupertinoIcons.list_bullet, size: 28),
        onPressed: () {
          showPopover(context: context, ...)
        }
    );
  }
}

but,the background mask covers pop-up window...

sorry,its my problem,however,can I set the barrierColor transparent

zhouxiaofu commented 2 years ago

Use StatefulBuilder to wrap widget

minikin commented 2 years ago

Sounds like we can close it. Feel free to reopen it if you think we can improve something. Thanks to everyone!

droidchief commented 5 months ago

The issue with the popover positioning is due to the context being used. Instead of using the context of the whole page, you need to get the context of the button itself to position the popover correctly. Here is an approach I used to ensure the popover is positioned relative to my button.

Steps:

1 Wrap the button with a Builder widget to get its context.

2 Use the context of the Builder widget to show the popover.

Builder( builder: (context) { return ElevatedButton( onPressed: () { showPopover( context: context, bodyBuilder: (context) => const Center( child: Text( "Hello Flutter!", )), width: 200, height: 100, backgroundColor: Colors.white, radius: 12, direction: PopoverDirection.top, ); }, child: const Text("Show popup")); }, )

Check out my solution