-
Notifications
You must be signed in to change notification settings - Fork 222
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
Add dynamicPosition prop to Position component #42
Add dynamicPosition prop to Position component #42
Conversation
Not sure why the tests are failing on TravisCI. When I run the karma tests locally everything passes. |
That was a Travis misconfig on our part. This looks good, except the name for the prop is a bit clunky. Not a blocker but does anybody have any better ideas? |
Good to know I didn't break anything! I had some trouble naming the prop. Another idea I had was autoPosition. On Sunday, November 29, 2015, Jimmy Jia notifications@github.com wrote:
|
I'd like to see this change merged so maybe a name like forceRepositionOnRender would be more accurate? It's a little verbose but better verbose than cryptic. |
Any other ideas on the name change for this property? |
Could just call it |
Well, then we'd have to manually rename it to |
f97de48
to
6b04520
Compare
Prop has been renamed to |
LGTM. @jquense? |
LGTM |
Add dynamicPosition prop to Position component
When it will be released? |
released |
This adds the
dynamicPosition
prop to thePosition
component in order to allow for the popover to be repositioned when content changes. It is turned off by default so it should not affect any existing code.This goes with this issue on react-bootstrap: react-bootstrap/react-bootstrap#1438