Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StatefulTable: OverflowMenu should scroll with its container #9082

Closed
ravinain opened this issue Jul 1, 2021 · 19 comments
Closed

StatefulTable: OverflowMenu should scroll with its container #9082

ravinain opened this issue Jul 1, 2021 · 19 comments
Assignees
Labels

Comments

@ravinain
Copy link

ravinain commented Jul 1, 2021

RowAction OverflowMenu in StatefulTable is not scrolling with the container.

This issue is similar to #4755 . However, I'm using StatefulTable

When I set the parent div position to relative, it fixes the scroll issue but introduces another. After scroll, the position of overflow menu is not correct. This is exactly same problem mentioned in the aforementioned ticket.

Dependency: carbon-addons-iot-react: 2.145
Browser: Chrome

Expected Behavior:
OverflowMenu should scroll with container. Also, after scroll the overflow menu should display at correct position.

Steps to reproduce the issue

  1. .parent div is scrollable
  2. scroll div
  3. Now click on overflow rowAction
  4. Notice the position of overflow rowAction menu

Example:

<div class="parent" data-floating-menu-container="data-floating-menu-container">
        <StatefulTable></StatefulTable>
</div>

.parent {
    position: relative;
}

You can also refer the aforementioned issue #4755.

@tw15egan
Copy link
Member

Hey there @ravinain, since this may be an issue with the StatefulTable component, would it make more sense to open an issue against the carbon-addons-iot-react repo instead? If you believe it is an issue with the core Carbon library, could you please recreate a reduced test case in CodeSandbox so we can take a look?

@ravinain
Copy link
Author

ravinain commented Aug 2, 2021

Hi,

I have created an example here: Overflow-Menu-Issue.

Steps to reproduce:

  • Click on overflow menu on any record
  • Scroll now. (The menu scroll with container, this is expected)
  • Click on any other record now. You will notice the menu opened but at incorrect position.

@tw15egan
Copy link
Member

tw15egan commented Aug 2, 2021

I'm unable to reproduce the issue.

2021-08-02 10 07 56

Going to close this as unable to reproduce, but if you can replicate the issue (without the StatefulTable component since that is an IOT add-on) we'd be more than happy to reopen and take another look.

@tw15egan tw15egan closed this as completed Aug 2, 2021
@petersandor
Copy link

@tw15egan this is still a problem, please see my slightly modified fork of the previously shared CodeSandbox: https://codesandbox.io/s/overflow-menu-issue-forked-y6f2d9

Nov-08-2022 12-45-40

@tw15egan
Copy link
Member

tw15egan commented Nov 8, 2022

@petersandor If this is an issue with the StatefulTable component, that would need to be fixed in the Carbon IOT add-ons repo. Are you able to reproduce the issue with the Carbon DataTable? I am still unable to reproduce it on our end.

@petersandor
Copy link

@tw15egan it doesn't matter what component the OverflowMenu is in (it can even be a plain div), I can set up a different example.

@tw15egan
Copy link
Member

tw15egan commented Nov 8, 2022

@petersandor, if you can reproduce the issue with just the overflow menu, please create a new issue so we can properly track a fix. This one seems to be tied to an addon library.

@petersandor
Copy link

@tw15egan see screen recording from Carbon React Storybook page:

Nov-08-2022 16-49-30

All that is needed is the overflow scroll on the parent container, height limitation and a lot of content before and after the overflow menu.

@tw15egan
Copy link
Member

tw15egan commented Nov 8, 2022

From slack:

May need to make sure the OverflowMenu has the data-floating-menu-container set to the parent selector (https://react.carbondesignsystem.com/?path=/docs/components-overflowmenu--default#data-floating-menu-container), but the storybook doesn’t have it set and I’m unable to see the same bug

2022-11-08 10 56 33

If a simplified reproduction can be made, we can work on a fix, but I am unaware of what is causing the issue.

@petersandor
Copy link

@tw15egan here's a new CodeSandbox with everything https://codesandbox.io/s/romantic-cloud-lbdm42?file=/src/index.js

Nov-08-2022 17-42-08

@tw15egan
Copy link
Member

tw15egan commented Nov 8, 2022

If you wrap the menu in a div and set that as the container, it should be contained inside the scrollable container.
https://codesandbox.io/s/dreamy-heisenberg-n41ncd?file=/src/index.js

@petersandor
Copy link

If you wrap the menu in a div and set that as the container, it should be contained inside the scrollable container. https://codesandbox.io/s/dreamy-heisenberg-n41ncd?file=/src/index.js

Right, so basically the overflow menu cannot have any sibling elements otherwise the positioning breaks.

@tw15egan
Copy link
Member

tw15egan commented Nov 8, 2022

It can have sibling elements; it just needs its position set to a container inside the scrolling container rather than the scrolling container itself.

@petersandor
Copy link

It can have sibling elements; it just needs its position set to a container inside the scrolling container rather than the scrolling container itself.

I see, this is an undocumented feature of the OverflowMenu though, I can see it is mentioned in the Tooltip which presumably uses the same underlying FloatingMenu component.

Thanks for help @tw15egan!

@tw15egan
Copy link
Member

tw15egan commented Nov 8, 2022

Sorry for the confusion! If we can improve the documentation regarding this functionality, I'd love to update it to reduce headaches in the future, just let me know what should be copied over from theTooltip documentation 🙂

@petersandor
Copy link

Actually if I may ask one more question, I noticed that the class is not really necesary for the data-floating-menu-container, it selects by the attribute only:

(triggerEl && triggerEl.closest('[data-floating-menu-container]')) ||

Which means it selects the overflow menu button itself (I confirmed by removing the attribute's value in your CodeSandbox example).

@petersandor
Copy link

@tw15egan isn't that an accessibility violation? The entire overflow menu is inside of the trigger button in your example? Also React complains that the overflow menu item (which is a button) is inside of the trigger button:

Screenshot 2022-11-08 at 19 14 10

@tw15egan
Copy link
Member

tw15egan commented Nov 8, 2022

Updated the example https://codesandbox.io/s/dreamy-heisenberg-n41ncd?file=/src/index.js so that it correctly appends to the div and not the OverflowMenu

#4755 (comment)

@petersandor
Copy link

Thanks again @tw15egan.

I still think that this is not ideal and the menu should handle this without forcing the consumers of the library to set position: relative and data-floating-menu-container attribute on the parent element as that may have other unintended side effects.

Other libraries solve this problem by dynamically recalculating the position of the menu which can also be tricky.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

No branches or pull requests

4 participants