ngneat / dialog

đŸ‘» A simple to use, highly customizable, and powerful modal for Angular Applications
https://ngneat.github.io/dialog/
MIT License
394 stars 38 forks source link

fix(draggable): move target instead of handle #92

Closed jemand771 closed 1 year ago

jemand771 commented 1 year ago

since the handle element is a child of the target element, it still gets moved as expected. this was probably more of a typo than an actual bug.

fixes #84

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

tl;dr: dragging a dialog handle only moves the handle and leaves the dialog in place. Issue Number: #84

What is the new behavior?

dragging the dialog handle moves the whole dialog (including the handle), as expected

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

stackblitz[bot] commented 1 year ago

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

jemand771 commented 1 year ago

I have no idea how to automatically test this. Since the draggable directive didn't have tests before and this bug was more of a typo than a logic issue, imo it's fine without tests (for now)

NetanelBasal commented 1 year ago

But what caused the issue? It worked in the past.

jemand771 commented 1 year ago

see this line in https://github.com/ngneat/dialog/commit/f5575f59da2fb61b9c0aa229e8890eaf32a2a56c

in some other places in that file, the (correct) change

- this.renderer.setStyle(this.handle, 'cursor', '');
+ this.handle.style.setProperty('cursor', '');

was made. but for the transform line, this happened:

-           this.renderer.setStyle(this.target, 'transform', transform);
+           this.handle.style.setProperty('transform', transform);

-> accidentally applying the transform to the handle instead of the target.

NetanelBasal commented 1 year ago

Thanks