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

3.2.0 Breaking Change: Drag Callbacks #391

Closed
STRML opened this issue Mar 1, 2019 · 6 comments
Closed

3.2.0 Breaking Change: Drag Callbacks #391

STRML opened this issue Mar 1, 2019 · 6 comments
Labels

Comments

@STRML
Copy link
Collaborator

STRML commented Mar 1, 2019

It appears there was an inadvertent breaking change in 3.2.0 relating to #361.

Prior to 3.2.0, if a defaultPosition were present, it became the x and y present in the callback on the initial drag and subsequent to that.

In 3.2.0, the x and y received in the callback is relative to the defaultPosition already passed. So if you pass a defaultPosition of x: 100, y:100and initiateonDragStart, the callback will contain {x: 0, y: 0}in the data. This makes sense if you considerdefaultPositionto be something more likeinitialOffset, and is required if your offset is something like, for example, {x: '10%', y: '10%'}.

Ref: bokuweb/react-rnd#437 and a broken build at https://circleci.com/gh/bokuweb/react-rnd/1340.

Next Steps

Our choices:

  • Is there a way to keep the old "absolute" offsets in callbacks from <= 3.1.1?
    • Percentage offsets will make this very difficult
  • Release a major version instead?
  • Drop the feature? A percentage-based default position can also be achieved by wrapping the <Draggable> in a div with a transform: translate(10%, 10%).

Ping @tnrich.

I've pulled 3.2.0 from npm and will be releasing a 3.2.1 with just the TS export fix.

@STRML STRML added the bug label Mar 1, 2019
@tnrich
Copy link
Contributor

tnrich commented Mar 1, 2019

A couple ideas @STRML to "keep the old "absolute" offsets in callbacks from <= 3.1.1?":

  • It seems like we could check to see if a percentage is being passed and only then do something more like an initial offset (this might be the most seamless as it would already work with existing packages)

  • could we introduce a new param, defaultPercentagePosition (or initialPosition). This would allow us to keep the current defaultPosition functionality the way it is. Only if you added a new property initialPosition would you get what the new functionality

If neither of the above ideas seem like viable solutions, then I think a major version would be most useful to my needs.

@STRML
Copy link
Collaborator Author

STRML commented Mar 2, 2019

I think a new param works, perhaps positionOffset - makes it clear that you're not pre-setting the state to something like {x: 100, y: 100}, you're offsetting the whole thing by some number or percentage and it doesn't actually affect the internal state and doesn't show up in callbacks.

@tnrich
Copy link
Contributor

tnrich commented Mar 8, 2019

@STRML I think that is a great plan! Do you need help implementing it?

@STRML
Copy link
Collaborator Author

STRML commented Mar 8, 2019

Yeah, that would be great if you have the time!

@tnrich
Copy link
Contributor

tnrich commented Mar 8, 2019

I've PR'd a first pass at doing this: #393

@tnrich
Copy link
Contributor

tnrich commented May 17, 2019

I think this can now be closed @STRML ?

@STRML STRML closed this as completed Sep 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants