Skip to content

refactor: let the target be passed into dragStart #1050

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.

Sign up for GitHub

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

Jump to bottom
Merged
daybrush merged 1 commit into from
Nov 26, 2023
Merged

refactor: let the target be passed into dragStart #1050

daybrush merged 1 commit into from
Nov 26, 2023

Conversation

@zsoerenm
Copy link
Contributor

@zsoerenm zsoerenm commented Nov 20, 2023

The event.target changes in asynchronous calls. It is therefore necessary to save the target at the desired time instance and pass it to dragStart.

Fixes: #1032

const target = e.target;
moveable.waitToChangeTarget().then(() => {
         moveable.dragStart(e, target);
   });

is an asynchronous call. e.target can, therefore, change. Fixing the target helps.

@zsoerenm
refactor: let the target be passed into dragStart
12c45bd 
The event.target changes in asynchronous calls. It is therefore necessary
to save the target at the desired time instance and pass it to dragStart.
@zsoerenm
Copy link
Contributor Author

zsoerenm commented Nov 21, 2023 •
edited
Loading

@daybrush
I added a second commit (8bb8293) that lets dragStart default to currentTarget instead of target.
This is especially useful for elements that consists of multiple sub-elements.
EDIT: This second commit is a breaking change. So you might want to merge only the first commit.

@zsoerenm zsoerenm force-pushed the ss/pass-target-to-dragstart branch from 8bb8293 to 12c45bd Compare November 22, 2023 06:57
@zsoerenm
Copy link
Contributor Author

zsoerenm commented Nov 22, 2023

I dropped the latest commit, because it is a breaking change and it fails tests. The currentTarget can by passed to dragStart manually anyway and, I think, it is better to not introduce a breaking change here.

@daybrush
Copy link
Owner

daybrush commented Nov 26, 2023

@zsoerenm

It's good code. I will reflect it.

@daybrush daybrush merged commit 786fc0c into daybrush:master Nov 26, 2023
@zsoerenm
Copy link
Contributor Author

zsoerenm commented Nov 26, 2023

@daybrush
would you mind making a new release?

@daybrush
Copy link
Owner

daybrush commented Dec 2, 2023 •
edited
Loading

@zsoerenm

Maybe release by monday of this week.

@daybrush
Copy link
Owner

daybrush commented Dec 3, 2023

@zsoerenm

  • croact-moveable 0.9.0
  • @moveable/helper 0.1.3
  • lit-moveable 0.30.0
  • moveable 0.53.0
  • preact-moveable 0.55.0
  • react-moveable 0.56.0
  • svelte-moveable 0.45.0
  • vue-moveable 2.0.0-beta.87
  • vue3-moveable 0.28.0
  • ngx-moveable 0.50.0

moveable's new version is released. Check it again.

@zsoerenm
Copy link
Contributor Author

zsoerenm commented Dec 4, 2023

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Is there a way to dragStart(e) a component that is different from e.target?

2 participants

@zsoerenm @daybrush