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

bug(cdkDrag): using cdkDragConstrainPosition causes erratic behavior #25154

Closed
1 task done
ma7moudat opened this issue Jun 24, 2022 · 23 comments
Closed
1 task done

bug(cdkDrag): using cdkDragConstrainPosition causes erratic behavior #25154

ma7moudat opened this issue Jun 24, 2022 · 23 comments
Labels
area: cdk/drag-drop P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@ma7moudat
Copy link

ma7moudat commented Jun 24, 2022

Is this a regression?

  • Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

13.3.9

Description

cdkDrag works normally with box and axis constraints, but as soon as cdkDragConstrainPosition is used, the dragged element incorrectly jumps a few pixels down the y-axis.

Reproduction

Please refer to the reproduction:

https://angular-ivy-iry1fk.stackblitz.io

https://stackblitz.com/edit/angular-ivy-iry1fk?file=package.json

Expected Behavior

The dragged element should only move horizontally and not jump down the y-axis.

Actual Behavior

The element jumps a few pixels down.

Environment

  • Angular: 14.0.0
  • CDK/Material: 14.0.0
  • Browser(s): Chrome / Firefox
  • Operating System (e.g. Windows, macOS, Ubuntu): Windows
@ma7moudat ma7moudat added the needs triage This issue needs to be triaged by the team label Jun 24, 2022
@crisbeto
Copy link
Member

This is likely the result of #25061 which made it so that the position constrain function determines the top/left coordinates of the element. You see it jump down a little because the "normal" dragging behavior also accounts for the pick up position within the element whereas the one returned from the constrain callback is the top/left. This is how the constrain position input was intended to work, but a poorly written unit test meant that it had broken at some point and we hadn't noticed.

@mj3052
Copy link
Contributor

mj3052 commented Jun 27, 2022

I must admit that I at this point cannot figure out how to use the cdkDragConstrainPosition after #25061 was merged. We had a working implementation with the old behavior.

I understand that the given point is now where the mouse is dragging on the page, and just returning the point will make the element render with the top/left at exactly that point. But even if I just offset that by say 10px, it seems to make the whole drag/drop experience act weird.

Maybe it is just missing documentation / information about how one is expected to implement the constraining logic I'm missing, but to me, the old behavior was at least a bit more intuitive.

@ma7moudat
Copy link
Author

ma7moudat commented Jun 27, 2022

This is likely the result of #25061 which made it so that the position constrain function determines the top/left coordinates of the element. You see it jump down a little because the "normal" dragging behavior also accounts for the pick up position within the element whereas the one returned from the constrain callback is the top/left. This is how the constrain position input was intended to work, but a poorly written unit test meant that it had broken at some point and we hadn't noticed.

Not sure I understand. Does that mean that it actually is a bug that needs fixing in cdkDrag? Or should I adjust my code to work with the new logic?

@crisbeto
Copy link
Member

The idea of that function has always been that the Point it returns is where you want the top/left corner of the dragged element to be. It's how it worked when I initially introduced it, but it regressed at some point. There was a unit test for it, but it was poorly written so it didn't catch the regression.

@ma7moudat
Copy link
Author

Ah okay! Thanks.
So if I understood correctly, it should work for me without the jumping if the Y of the returned point matches the getBoundingClientRect().top of the parent element (or the constraining element).

@ckorherr
Copy link

@crisbeto I have the same problem, basically I want to use it for a split view, so I lock the axis in one direction and also want to constrain the position in the other direction. This is now next to impossible and is preventing us from updating to Angular 14. With Angular 13 it worked perfectly.

This is a simple example showing the problem:
https://stackblitz.com/edit/angular-zsuwad-juyc6z?file=package.json,src%2Fapp%2Fcdk-drag-drop-axis-lock-example.html,src%2Fapp%2Fcdk-drag-drop-axis-lock-example.ts

With axes locked, the constraint causes erratic behavior.

@crisbeto
Copy link
Member

So the (not well documented) assumption is that if you're using constrainPosition, you're in complete control over the positioning. lockAxis and boundary may end messing with it since they're all modifying the same coordinates and can end up returning conflicting information.

In retrospect, lockAxis and boundary should've been implemented as functions for constrainPosition that are provided by cdk/drag-drop.

@ckorherr
Copy link

Makes sense, I'll try that, thank you!

@ckorherr
Copy link

Works perfect.

@ioanes
Copy link

ioanes commented Jul 25, 2022

The idea of that function has always been that the Point it returns is where you want the top/left corner of the dragged element to be. It's how it worked when I initially introduced it, but it regressed at some point. There was a unit test for it, but it was poorly written so it didn't catch the regression.

Can you provide an example how to calcuate returning point coordinates so it will drag the item without jumping to top left corner? So basicply the constrainPosition function will not do anything. From that point we can adjust the code for our needs.

`constrainPosition(point: Point, dragRef: DragRef)
{

// calculate default beahviour. Move element from where you grab it.

return point;

}`

Thank you

crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 26, 2022
Exposes the pickup position within the element to the `constrainPosition` callback so that users can account for it in their calculations.

Relates to angular#25154.
@crisbeto
Copy link
Member

You'd have to offset it based on the position at which the user picked up the element, which I now realize isn't possible to read. I've opened up #25341 to expose it within the constrainPosition callback.

crisbeto added a commit that referenced this issue Jul 26, 2022
…ack (#25341)

Exposes the pickup position within the element to the `constrainPosition` callback so that users can account for it in their calculations.

Relates to #25154.
crisbeto added a commit that referenced this issue Jul 26, 2022
…ack (#25341)

Exposes the pickup position within the element to the `constrainPosition` callback so that users can account for it in their calculations.

Relates to #25154.

(cherry picked from commit 73c0c60)
crisbeto added a commit that referenced this issue Jul 26, 2022
…ack (#25341)

Exposes the pickup position within the element to the `constrainPosition` callback so that users can account for it in their calculations.

Relates to #25154.

(cherry picked from commit 73c0c60)
@kvetis
Copy link

kvetis commented Aug 19, 2022

Watch out guys/gals, just spent 2 hours debugging why my dragged element jumps like crazy and the reason is that you receive user pointer position and you're supposed to return top left corner position. So I guess not much has changed and still have to use pickup position in the calculations. At least now I no longer have to calculate it but receive it as the last function. It would be nice to actually receive the top left corner point, apply limiting function and return top left corner point.

@kvetis
Copy link

kvetis commented Aug 23, 2022

@ioanes To get the top left corner from the point, you need to subtract the pickup position in draggable from the Point you get as the first argument:

/** Fullfills cdkDragPosition input for constraining drag element. */
  constrainPosition: CdkDrag['constrainPosition'] = (
    { x, y }: Point,
    _ref,
    _dims,
    // You get the pickup position which is the offset from the top left corner of the image
    pickup: Point,
  ) => {

    // Keep whatever logic you had before v14.
   
    // Convert x and y to top left corner of the image
    x -= pickup.x;
    y -= pickup.y;

    return { x, y };
  };

You need to be on the latest version though, since @crisbeto added the pickup parameter a month ago. Before that, you had to calculate it yourself.

@ioanes
Copy link

ioanes commented Aug 23, 2022

Thanks for example code @kvetis ;)

Thats why I asked, since pickup position wasnt available in constrainPosition at that time and calculating it by myself when it can be exposed like @crisbeto did was a perfect solution.

Also Thanks @crisbeto for a quick response and fix.

@HuntosShuntos
Copy link

This bug also seems to impact other parts of cdkDrag. Specifically when cdkDragConstrainPosition when used with cdkDragBoundary also seems to have erratic behavior.

Reproduction
See below
https://angular-ivy-9unppr.stackblitz.io
https://stackblitz.com/edit/angular-ivy-9unppr?file=src/app/app.component.html

Environment

Angular: 15.0.3
CDK/Material: 15.0.3
Browser(s): Chrome / Firefox
Operating System (e.g. Windows, macOS, Ubuntu): Windows

Expected Behavior
Should stay within the drag boundary and be able to go to the edges of the boundary

Actual Behavior
Seems that the drag boundary arbitrarily moves

@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent area: cdk/drag-drop and removed needs triage This issue needs to be triaged by the team labels May 4, 2023
@mj3052
Copy link
Contributor

mj3052 commented Aug 28, 2023

@crisbeto

Is there anything I/someone can do to get this moving forward somehow? Or at least get some clarification about what is going to happen with the strange behavior still seen in some cases. We are currently stuck building our own patched version using the old behavior before #25061 for our project, as we're using cdkDragBoundary with cdkDragConstrainPosition in quite a few places.

I am open to contribute in solving this if required. It seems the example of @HuntosShuntos is showing the problem we're having, which was introduced by #25061.

If cdkDragBoundary is not going to work together with cdkDragConstrainPosition I think it should at least be documented that this is the case.

@crisbeto
Copy link
Member

@mj3052 I haven't had time to look into this, because of different team priorities. I'd be open to reviewing and landing a fix though.

@PentoreXannaci
Copy link

i stumbled across this problem too, i thought i go crazy until i saw this issue!
here is my example: https://stackblitz.com/edit/hvp9cm

i cant move the element to the top left corner. (somehow its depending on where i click on the element).

IMO, this is a huge issue and should get a higher priority.

@leanix-micwel
Copy link

This is still blocking us from updating as well. We are running angular 15 with cdk 13 for now.

@mj3052
Copy link
Contributor

mj3052 commented Aug 30, 2023

I've tried to implement an initial fix in PR #27730. All tests seem to be passing.

@crisbeto Let me know what needs to be done in order to get this further. I am not sure about how you guys normally use the dev-app, but I added two more cases in it (one using just the boundary and one combining the boundary with a constraint), to make sure it works in all cases.

I can easily remove this if need be.

@marvinrohde-leanix
Copy link

marvinrohde-leanix commented Dec 4, 2023

@mj3052 Now working perfectly fine (tested in 16.2.12). Thanks so much for fixing this! 🥇

@crisbeto
Copy link
Member

Closing since this should be fixed now.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: cdk/drag-drop P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

No branches or pull requests

10 participants