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

Overflow menu doesn't scroll with page #911

Closed
travis1111 opened this issue Jun 19, 2018 · 31 comments
Closed

Overflow menu doesn't scroll with page #911

travis1111 opened this issue Jun 19, 2018 · 31 comments

Comments

@travis1111
Copy link

Detailed description

I have a page has many cards, there is a overflow menu (three vertical dots) on each card. I can click on the menu (three dots) to bring the drop down menu. However if the page has scroll bar (too many cards to fit into one page), when I scroll down, the drop down menu stays where it was, it won't scroll with the card it attached too.

Is this issue related to a specific component?

Overflow menu

What did you expect to happen? What happened instead? What would you like to see changed?

Should scroll with the menu icon (three dots)

What browser are you working in?

Chrome

What version of the Carbon Design System are you using?

Latest

What offering/product do you work on? Any pressing ship or release dates we should be aware of?

Microclimate

Steps to reproduce the issue

see description;

Additional information

  • Screenshots or code
  • Notes

Add labels

Please choose the appropriate label(s) from our existing label list to ensure that your issue is properly categorized. This will help us to better understand and address your issue.

@travis1111
Copy link
Author

image

@alisonjoseph
Copy link
Member

Can you create a codepen to show the issue you are having. I am unable to duplicate. Thanks! https://codepen.io/team/carbon/full/MOEwjp/

@tw15egan tw15egan changed the title OVerflow menu doesn't scroll with page Overflow menu doesn't scroll with page Jun 20, 2018
@travis1111
Copy link
Author

and probably similar issue:

its a screen recording, you need to unzip and play. Github doesn't allow me to attach videos.
bug.mp4.zip

@travis1111
Copy link
Author

You won;t be able to reproduce it if you just use stock sample. See my screen shot, I have cards, at least 4 cards, and make the screen just fit for 3 and try it. Please can you take a look of the video I attached above.

@tw15egan
Copy link
Collaborator

tw15egan commented Jun 22, 2018

Can you recreate the sample in something like codesandbox or codepen? I see the issue, but without actually being able to look at the code there is not much we can do to help

@travis1111
Copy link
Author

travis1111 commented Jun 25, 2018

Hi, it turns out something to do with overflow-y: auto settings on the div we have.

So we have something like this

header here
<div style="overflow-y:auto;">
...
...
card with overflow menus, many of them

</div>
footer

The result is, we create a sticky header and footer, so that only the div with overflow-y:auto is scrollable. The overflow menu is in that scrollable view. When we click the three dots, the menus (ul wiith li s) are created outside of that div, so i think that is the why the overflow menu doesn't scroll with the content of the div.

So is there a way to let overflow menu to be placed inside the div, e.g. close to the element contains three dots so that it could stay with the 3 dots menu when scrolling?

@asudoh
Copy link
Contributor

asudoh commented Jun 25, 2018

@travis1111 Currently not - We know there is a need for “custom floating menu target” mechanism, though, e.g. #910 - Will see if we can come up with something simple.

@travis1111
Copy link
Author

Thanks @asudoh also did you see the video I pasted above? bug.mp4.zip, I think it is also related, the ul is placed outside of the scrollable area.

Do you have a rough timeline for when this can be fixed?

@asudoh
Copy link
Contributor

asudoh commented Jun 26, 2018

@travis1111 Not at this moment - Meanwhile, feel free to copy overflow-menu.js and floating-menu.js code over to your project and customize them as your project desires. Thanks!

@asudoh
Copy link
Contributor

asudoh commented Jun 28, 2018

Oops I totally forgot that the floating menu code looks at an element with data-floating-menu-container attribute first before looking at <body>. Hope it helps some.

asudoh added a commit to asudoh/carbon-components-react that referenced this issue Jun 28, 2018
This change makes `<FloatingMenu>` support an element to put the menu into,
which is, the ancestor of the trigger buttun with `data-floating-menu-container` attribute.

This change also removes code for React15-, which we dropped the support from 9.x codebase.

Refs carbon-design-system/carbon#910.
Refs carbon-design-system/carbon#911.
asudoh added a commit to asudoh/carbon-components-react that referenced this issue Jun 28, 2018
This change makes `<FloatingMenu>` support an element to put the menu into,
which is, the ancestor of the trigger button with `data-floating-menu-container` attribute.

This change also removes code for React15-, which we dropped the support from 9.x codebase.

Refs carbon-design-system/carbon#910.
Refs carbon-design-system/carbon#911.
marijohannessen pushed a commit to carbon-design-system/carbon-components-react that referenced this issue Jun 29, 2018
This change makes `<FloatingMenu>` support an element to put the menu into,
which is, the ancestor of the trigger button with `data-floating-menu-container` attribute.

This change also removes code for React15-, which we dropped the support from 9.x codebase.

Refs carbon-design-system/carbon#910.
Refs carbon-design-system/carbon#911.
@asudoh
Copy link
Contributor

asudoh commented Jul 3, 2018

Wrt the movie in the zip - It appears different from the issue with scrolling this PR is taking about. Feel free to open a separate issue if the problem still exists. Thanks!

@asudoh asudoh closed this as completed Jul 3, 2018
@travis1111
Copy link
Author

@asudoh thanks. how do I use this attribute? something like this?

<div class="parent">
<div data-overflow-menu data-floating-menu-container=".parent" .....

@asudoh
Copy link
Contributor

asudoh commented Jul 5, 2018

@travis1111 You are right assuming the 1st <div> contains the 2nd <div>.

@travis1111
Copy link
Author

hmm then it is not working...

here is the demo code (I forked from your official codepen):

https://codepen.io/anon/pen/XYwoVM

@asudoh
Copy link
Contributor

asudoh commented Jul 6, 2018

Oops I had a oversight looking at your HTML - It should be:

<div data-floating-menu-container>
  <div data-overflow-menu ...>
    ...
  </div>
</div>

@travis1111
Copy link
Author

@asudoh Hi, i tried this

<div data-floating-menu-container> ... ...

Still not working. here is an example:

https://codepen.io/anon/pen/QBgbVJ/

@asudoh
Copy link
Contributor

asudoh commented Jul 24, 2018

Floating menu uses position:absolute. It means that it's positioned with respect the nearest ancestor with a position other than static (refs: https://www.w3.org/TR/css-position-3/#comp-abspos and https://www.w3.org/TR/css-position-3/#containing-block). Adding position:relative to the div.data-floating-menu-container in your CodePen seems to fix the issue you described. Hope it helps.

@travis1111
Copy link
Author

thanks. but the y position is off... I added one more, you can see y position is doubled...

https://codepen.io/anon/pen/QBgbVJ/

@asudoh
Copy link
Contributor

asudoh commented Jul 25, 2018

There is an option in overflow menu for you, which is objMenuOffset. An object with top and left properties, or a function returning such object can be set there. In this way you can adjust the overflow menu’s position tailored for your application’s structure. Options in Carbon vanilla classes/instances can be set via the second argument when you instantiate it (the .create() API: https://github.com/IBM/carbon-components/tree/53bfbcf/src/globals/js/mixins#creation). Hope it helps.

@travis1111
Copy link
Author

travis1111 commented Jul 26, 2018

Thanks I have made this working

https://codepen.io/anon/pen/QBgbVJ/

But I don't understand why I had to add a callback to adjust the position. Also do you mind check if the code I have on codepen makes sense?

Also I used the same code (as above) in our project, the objMenuOffset callback is not called at all. Is there anything preventing it from calling?

@asudoh
Copy link
Contributor

asudoh commented Jul 26, 2018

Hi @travis1111 good question - What floating menu, the underlining mechanism of overflow menu, does is getting the position of the trigger button relative to the top/left corner of the viewport via CSSOM API and setting the top/left position of the menu based on that. Though we know that this is not something that will work for every usage pattern of floating menu, we also know that an attempt to support every usage pattern is almost impossible and tend to cause bloated, error-prone code. From that reason, what we do is setting the position that works for the most common pattern and provide an API to adjust the menu position.

BTW I see objMenuOffset callback is called in your CodePen. Note that it's called when the menu opens.

@travis1111
Copy link
Author

Thanks @asudoh

Yes, the callback in codepen works, but it did not work in our project. Is there any settings to prevent it from calling? is the callback only available in latest version?

@asudoh
Copy link
Contributor

asudoh commented Jul 27, 2018

@travis111 The most possible cause is the component was instantiated already before you tried to instatiate it with the option. Static components property has all the instances mapped with DOM elements and you may want to look into that.

@travis1111
Copy link
Author

Thanks but I have this code

parent.insertAdjacentHTML ("beforeend",html);
        var offset = function () {
        	    console.log("in offset");
        	    return {top:0, left:100};
        }
        var options = { objMenuOffset: offset };
        
        CarbonComponents.OverflowMenu.create(document.getElementById("overflowmenu-" + project.projectID), {objMenuOffset: offset });

and if I comment out CarbonComponents.OverflowMenu.create.... then when I click the overflow menu (3 dots), nothing happens...

@asudoh
Copy link
Contributor

asudoh commented Jul 27, 2018

@travis1111
Copy link
Author

Still don't know why...

this is what shows in the debugger of my project:

image

and step in:

image

and this is what shows in codepen:

image

When the _changeState.apply is called, the codepen source has the correct offset object which is a callback I defined, but in my project it is replaced by getMenuOffset from carbon component.

@asudoh
Copy link
Contributor

asudoh commented Jul 27, 2018

@travis1111 You may want to debug the .create() then: https://github.com/IBM/carbon-components/blob/0336425/src/globals/js/mixins/create-component.js#L45

Whether or not your option is applied or not will be determined by if there is an existing instance, as the code shows.

@travis1111
Copy link
Author

@asudoh I changed the source code and ran it in the debugger, no existing components, as you can see from the debugger below. So what I do next?

image

@asudoh
Copy link
Contributor

asudoh commented Jul 30, 2018

@travis1111 I see you are using “flip mode” of floating menu. My apologies not imagining you using that, but the option to use for flip mode is objMenuOffsetFlip instead of objMenuOffset: https://github.com/IBM/carbon-components/blob/53bfbcf/src/components/overflow-menu/overflow-menu.js#L117-L119

@travis1111
Copy link
Author

thanks change to objMenuOffsetFlip now it calls the call back, but the position is still not right. When are you going to be online tomorrow? do you mind I sametime you?

@asudoh
Copy link
Contributor

asudoh commented Aug 1, 2018

Hi 👋 I recently got a document update, which you may want to refer to: https://github.com/IBM/carbon-components/tree/ad19b01/src/components/overflow-menu#example---changing-menu-position-by-8-pixels-vertically

You can search for channels with Carbon names in our internal communication tool to reach out to us internally.

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

No branches or pull requests

4 participants