mantinedev / mantine

A fully featured React components library
https://mantine.dev
MIT License
26.92k stars 1.9k forks source link

`Popover` with `withinPortal` props won't scroll in scrollable element #3351

Closed congyaqwq closed 1 year ago

congyaqwq commented 1 year ago

What package has an issue

@mantine/core

Describe the bug

2 issues In a scrollable element

  1. Popover.Dropdown won't scroll with Popover.Target without position: relative;
  2. Popover with withinPortal props won't scroll in scrollable element even it's position is relative.

What version of @mantine/hooks page do you have in package.json?

5.8.2

If possible, please include a link to a codesandbox with the reproduced problem

https://codesandbox.io/s/gifted-bessie-im2ocp?file=/src/App.tsx

Do you know how to fix the issue

Yes

Are you willing to participate in fixing this issue and create a pull request with the fix

Yes

Possible fix

Before 5.1.0 maybe, there comes no bug with Popover( with no need for position: relative ); Maybe problem in file useFloatingAutoUpdate.tsx also, flip middleware should use before shift WX20230112-192159

from https://floating-ui.com/docs/flip

rtivital commented 1 year ago

You are welcome to submit a PR with a fix

wes337 commented 1 year ago

@congyaqwq can you help me understand this issue better?

In your codesandbox, everything seems to be scrolling correctly. The page won't scroll when the cursor is inside the popover, but isn't this the correct behavior?

And I do not see any effect on when un-commenting line 16 (position: relative)

congyaqwq commented 1 year ago

@congyaqwq can you help me understand this issue better?

In your codesandbox, everything seems to be scrolling correctly. The page won't scroll when the cursor is inside the popover, but isn't this the correct behavior?

And I do not see any effect on when un-commenting line 16 (position: relative)

Sorry, I tried to solve this problem by finding which version it changed, now it's version is 5.8.2, you can see it 's behavior

maradney commented 1 year ago

@congyaqwq can you help me understand this issue better? In your codesandbox, everything seems to be scrolling correctly. The page won't scroll when the cursor is inside the popover, but isn't this the correct behavior? And I do not see any effect on when un-commenting line 16 (position: relative)

Sorry, I tried to solve this problem by finding which version it changed, now it's version is 5.8.2, you can see it 's behavior

@congyaqwq Are you sure about this working before 5.1.0 ? Can you provide a codesandbox where it works? as I tried it and it seems to be persistent.

congyaqwq commented 1 year ago

@congyaqwq can you help me understand this issue better? In your codesandbox, everything seems to be scrolling correctly. The page won't scroll when the cursor is inside the popover, but isn't this the correct behavior? And I do not see any effect on when un-commenting line 16 (position: relative)

Sorry, I tried to solve this problem by finding which version it changed, now it's version is 5.8.2, you can see it 's behavior

@congyaqwq Are you sure about this working before 5.1.0 ? Can you provide a codesandbox where it works? as I tried it and it seems to be persistent.

https://codesandbox.io/s/zealous-ellis-05egsk?file=/src/App.tsx:417-429 In version 5.10.1, It works. I test it in codesandbox, but in my local env, It still did not work.

https://user-images.githubusercontent.com/45482161/219676314-9c4e4e62-03c5-464f-8b03-f988b2ca6970.mov

Thank you for your help.

maradney commented 1 year ago

@congyaqwq I found the issue and here is a PR that fixes it https://github.com/mantinedev/mantine/pull/3576, let me know if it works with you 🙏

I didn't put flip middleware before shift as it didn't affect this issue but we should address it too just in case

congyaqwq commented 1 year ago

@congyaqwq I found the issue and here is a PR that fixes it #3576, let me know if it works with you 🙏

I didn't put flip middleware before shift as it didn't affect this issue but we should address it too just in case

yep, It works, thanks !