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

Confusing documentation with boundaries (how to ensure flip with floating navbar?) #518

Closed
niftylettuce opened this issue Jan 7, 2018 · 16 comments
Labels
feature This would be nice to have. TARGETS: modifier Related to a modifier.

Comments

@niftylettuce
Copy link

I have a .navbar-fixed-top (using Bootstrap 3 and implementing a custom Popper instance).

Trying to figure out how to make it so when you scroll down the page, a popover that begins to be hidden underneath a fixed top navbar will get flipped automatically.

I tried passing a modifiers.flip.boundariesElement option, but that didn't seem to work.

Is there any way to do a simple "reduce viewport top size by X pixels?" where X is the height of the floating navbar?

@FezVrasta
Copy link
Member

You should define an offset.

@niftylettuce
Copy link
Author

@FezVrasta how do I do that btw? It says its an Object, but is it offset.top, etc? I couldn't find it anywhere in docs and started to search in source code..

@mwebb33
Copy link

mwebb33 commented Feb 14, 2018

I am also confused by the offset implementation/documentation. Sorry, I tried. I got frustrated.

@mwebb33
Copy link

mwebb33 commented Feb 14, 2018

I basically want to set a pixel offset from the top of the viewport and the left of the viewport.

@FezVrasta FezVrasta added # QUESTION ❔ docs No code, just documentation. labels Apr 3, 2018
@beowulfenator
Copy link

Same problem here. @FezVrasta, can you please be more specific about there to define the offset? Thanks!

@FezVrasta
Copy link
Member

FezVrasta commented Apr 11, 2018

Sorry I thought Popper.js already supported it but it seems like it doesn't.

Ideally we should have an option for preventOverflow and flip that allows to set offsets on the 4 sides, it shouldn't be difficult to implement.

As workaround you may have your layout structured like this:

<body>
  <header />
  <main />
</body>

One below the other, so that you can set main as boundariesElement.

I haven't tried it, but I'm quite sure you may also provide as boundariesElement a custom referenceObject-like element with the size of the area you want to constrain the popper to.

Sorry for the confusion.

@FezVrasta FezVrasta added feature This would be nice to have. TARGETS: modifier Related to a modifier. and removed # QUESTION ❔ docs No code, just documentation. labels Apr 11, 2018
@BrianBu01
Copy link

BrianBu01 commented Apr 12, 2018

A real simple way to fix this is to change lines 72-75 in getBoundaries.js lines to:

boundaries.left += padding.left || padding;
boundaries.top += padding.top || padding;
boundaries.right -= padding.right || padding;
boundaries.bottom -= padding.bottom || padding;

@FezVrasta
Copy link
Member

FezVrasta commented Apr 12, 2018

@BrianBu01 feel free to fork this repo, do your changes and send a pull request. Thanks! 🤩

BrianBu01 pushed a commit to BrianBu01/popper.js that referenced this issue Apr 12, 2018
@beowulfenator
Copy link

@BrianBu01 This didn't help in my case. Am I doing something wrong? How does this update work?

@BrianBu01
Copy link

BrianBu01 commented Apr 12, 2018

For me after the change my code using Popper would be:

var popper = new Popper(
            target, 
            target.siblings()[0], {
              modifiers: {
                preventOverflow: { 
                  enabled: true,
                  padding: {
                    bottom: 75
                  }
                },
              }
            });

@beowulfenator
Copy link

Thank you! This solves my problem. For the default 50px tall bootstrap navbar the code looks like this:

var popper = new Popper(referenceElement, popperElement, {
  modifiers: {
    flip: {
      padding: {
        top: 50
      }
    }
  }
});

Can't wait to see your PR pulled in!

BrianBu01 pushed a commit to BrianBu01/popper.js that referenced this issue Apr 13, 2018
@BrianBu01
Copy link

@beowulfenator I made additional changes to that method which are pending.

BrianBu01 pushed a commit to BrianBu01/popper.js that referenced this issue Apr 18, 2018
BrianBu01 pushed a commit to BrianBu01/popper.js that referenced this issue Apr 19, 2018
BrianBu01 pushed a commit to BrianBu01/popper.js that referenced this issue Apr 19, 2018
BrianBu01 pushed a commit to BrianBu01/popper.js that referenced this issue Apr 19, 2018
BrianBu01 pushed a commit to BrianBu01/popper.js that referenced this issue Apr 19, 2018
BrianBu01 pushed a commit to BrianBu01/popper.js that referenced this issue Apr 19, 2018
BrianBu01 pushed a commit to BrianBu01/popper.js that referenced this issue Apr 19, 2018
BrianBu01 pushed a commit to BrianBu01/popper.js that referenced this issue Apr 19, 2018
BrianBu01 pushed a commit to BrianBu01/popper.js that referenced this issue Apr 19, 2018
BrianBu01 pushed a commit to BrianBu01/popper.js that referenced this issue Apr 19, 2018
BrianBu01 pushed a commit to BrianBu01/popper.js that referenced this issue Apr 19, 2018
BrianBu01 pushed a commit to BrianBu01/popper.js that referenced this issue Apr 19, 2018
grfullerton added a commit to teammgl/popper.js that referenced this issue Apr 24, 2018
feat: Add specific padding to boundaries (floating-ui#518)
BrianBu01 pushed a commit to BrianBu01/popper.js that referenced this issue Apr 25, 2018
BrianBu01 pushed a commit to BrianBu01/popper.js that referenced this issue Apr 25, 2018
@BrianBu01
Copy link

Here is a codepen just in case someone needs it.

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

@kasparsuvi1
Copy link

Hey! Thanks for the update.

Unfortunately it gives error with typescript. Because types aren't updated.
flip and preventOverflow padding types should be updated to accept object of offsets.

@FezVrasta Is there any hope to get it fixed?

@FezVrasta
Copy link
Member

@kasparsuvi1 I'm not a TypeScript user, but feel free to send a PR to fix that

@kasparsuvi1
Copy link

Made PR #750 for that.
@FezVrasta

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This would be nice to have. TARGETS: modifier Related to a modifier.
Projects
None yet
Development

No branches or pull requests

6 participants