-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add offsetParent
prop for alternative offset calculations
#173
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
Conversation
afdb663
to
7b27283
Compare
The issue with the failing tests is that React has changed the way propType warnings are being thrown. It now provides a backtrace. On
The only way I can think of right now is to replace |
The tests are fixed by #176 |
ah thanks, I will rebase once merged |
Thanks for this, I am reviewing. I'm looking for a simpler solution, though. Will get back to you shortly. |
I'd prefer an |
@STRML sounds good, will do! |
c0dbe54
to
29d35b2
Compare
Ok, it's rebased and `offsetParent chang eis included, thanks for the review :) |
* `offsetParent`, if set, uses the passed DOM node to compute drag offsets | ||
* instead of using the parent node. | ||
*/ | ||
offsetParent: PropTypes.bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a bool. Unfortunately there's no dom node propType so it should be something like:
offsetParent: function(props, propName) {
if (process.browser && props[propName] && props[propName].nodeType !== 1) {
throw new Error("Draggable's offsetParent must be a DOM Node.");
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, sorry I overlooked when changing that.
Please rebase again as #176 was merged. |
9c553e6
to
8c2b209
Compare
useBodyAsOrigin
to <DraggableCore>offsetParent
prop for alternative offset calculations
8c2b209
to
33e0a04
Compare
…Core> Allows nodes to use an arbitrary node as origin instead of the parent. This is useful, when the parent's position is changing. When used, resolves react-grid-layout#170 This issue was introduced by a398097
33e0a04
to
3d98be3
Compare
Sorry, I updated the code quite hasty previously. Everything should be ok now, I hope. Thanks for your patience |
const offsetParentRect = node.offsetParent === document.body ? {left: 0, top: 0} : offsetParent.getBoundingClientRect(); | ||
export function offsetXYFromParentOf(evt: {clientX: number, clientY: number}, node: HTMLElement & {offsetParent: HTMLElement}, draggableCore: DraggableCore): ControlPosition { | ||
const offsetParent = draggableCore.props.offsetParent || node.offsetParent || document.body; | ||
const offsetParentRect = offsetParent ? {left: 0, top: 0} : offsetParent.getBoundingClientRect(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition will always be true.
I'm going to refactor this a bit and push. Thanks for the contribution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right this should be offsetParent === document.body
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, but we can't even use document.body, we have to use node.ownerDocument
.
Almost done, just writing a test to verify.
Pushed in #179! |
Thanks @STRML !! |
Hi everybody, can someone help me please how to use this feature? How can i pass html node, when it is not rendered yet?
Thanks |
Like you pointed out, you need the parent to have been mounted before you can pass the node as setRef = (ref) => {
this.rootRef = ref;
this.forceUpdate();
},
render() {
// First comes your parent html; I just put a div as an example
return (
<div className="parent" ref={this.setRef}>
{ this.rootRef ? (
<GraphElement
id={element.id}
key={element.id}
posX={element.cords.x}
posY={element.cords.y}
parentOffset={this.rootRef}
onElementJointDragStart={() => this.props.onJointDragStart(element.id)}
onElementJointDragMove={(event, data) => this.props.onJointDragMove(element.id, { x: data.x, y: data.y })}
/>
) : null }
</div>
);
} |
Thank you for quick replay. It works. I just added a condition to prevent a loop.
But I am not sure if this is the correct way how to work with react. |
Usually you do something like this: componentDidMount() {
this.setState({mounted: true});
}
render() {
...
ref="graphElement"
parentOffset={this.state.mounted ? this.refs.graphElement : null}
...
} |
String refs are discouraged because of lack of composability / HOC flexibility (reference) |
Also wonder whether you actually need to pass the parentOffset. The drag handler will also receive onDrag (ev, data) {
....
const coords = {
x: old.x + data.deltaX,
y: old.y + data.deltaY
}
} |
Thank you all for your tips and tricks. I think ref should be a function. Toggle the mounted variable in state seems to be a good solution. I need this cause I have draggable elements within another draggable element and all shoud have the same parentOffset. |
Allows nodes to use the body as origin instead of the parent. This is useful, when the parent's position is changing. When used, resolves #170
This issue was introduced by a398097
please 👀